[cxx-interop] ClangDeclFinder to handle problematic codegen cases

[plotfi]

At the moment, header defined functions do necessarily get codegened by
C++-Interop if they are invoked purely on the call path from a
constructor. What this means is that when you have inline or template
functions that are header defined and are only invoked on a call graph
path that only originates from a C++ constructor that is invoked from
Swift, that you get LLVM IR Function declares instead of Function
defines with linkonceodr. This results in link errors unless those
functions happen to be invoked or template instantiated from another TU
that gets linked in.

This patch teaches the ClangDeclFinder to look for these cases and add
them to the list of decls that get handled by HandleTopLevelDecl.

Partly addresses SR-15272

[plotfi]

@zoecarver @egorzhdan @hyp I'd appreciate any comments or considerations here. I intend to continue expanding on this patch, but this is what I have that partly works for now.

[zoecarver]

Looks pretty good. A few comments, but I want to look into possibly factoring this into the existing ClangDeclFinder.

Thanks for the patch!

[zoecarver]

It's too bad that the existing ClangDeclFinder can't be used/extended here. I'm going to look into this API a bit more to see if we could potentially re-use anything.

[zoecarver]

General notes for the tests:

1. Let's match the existing naming convention. TitleCase for classes and camelCase for functions.
2. Name the functions based on what they do. So this might be named returns42 and the next one might be passThroughArg.
3. header-defined-functions doesn't really explain what's being tested here. This is testing when a data member's initializer uses an inline function, so maybe this should go in Cxx/class/initializer-from-inline.h.

[plotfi]

@zoecarver I've addressed most of the review feedback. Ready for more.

[plotfi]

@zoecarver Current diff handles most of the std::vector case as well. I think one big thing I was doing wrong was not picking up a decl if the method body was empty. As this point the only missing linkonceodr function is the destructor. Having some trouble figuring out what the right plumbing is to invoke CGM.EmitGlobalDefinition(GD, GV)

[plotfi]

#40867 is a much nicer and more complete solution. Closing.

[bulbazord]

#40867 has been in the tree for some time so I tried the examples in the description of SR-15272, notably the one involving std::vector. I'm still getting linker errors there. I'm wondering if this patch should be revisited? I pulled this patch into my tree and tested both examples in the bug report, both of them link and run correctly (on my machine).

[zoecarver]

#40867 has been in the tree for some time so I tried the examples in the description of SR-15272, notably the one involving std::vector. I'm still getting linker errors there. I'm wondering if this patch should be revisited? I pulled this patch into my tree and tested both examples in the bug report, both of them link and run correctly (on my machine).

I'd be interested to explore this more. Are you using a just-built Swift compiler? Do you know what APIs aren't linking? Maybe you can provide the input program you're using?

[bulbazord]

I'd be interested to explore this more. Are you using a just-built Swift compiler? Do you know what APIs aren't linking? Maybe you can provide the input program you're using?

Sure, I'll provide as many details as I can.

I effectively merged the 2 programs referenced in SR-15272. Here's what I have (all files in the same directory):

A.h

inline int Answer() { return 42; }
struct A { int a = Answer(); };

Vec.h

#include <vector>
struct Vec {
  std::vector<int> v = {1, 2, 3, 4};
};

module.modulemap

module A {
  header "A.h"
  requires cplusplus
}

module Vec {
  header "Vec.h"
  requires cplusplus
}

test.swift

import A
import Vec

import std.vector

var a = A()
var answer = a.a

var v = Vec()
var Vector = v.v

for i in 0 ..< Vector.size() {
    print(Vector[i], terminator: " ")
}

I then built test.swift with the following command:
xcrun --toolchain default --sdk '$SDK_DIR/MacOSX12.1.sdk' $SWIFT_BUILD_DIR/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/bin/swiftc -target x86_64-apple-macosx10.9 -module-cache-path $SWIFT_BUILD_DIR/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache -F '$XCODE_FRAMEWORKS_DIR' -toolchain-stdlib-rpath -Xlinker -rpath -Xlinker '$XCODE_FRAMEWORKS_DIR' -Xlinker -rpath -Xlinker /usr/lib/swift -Xlinker -headerpad_max_install_names -F '$SWIFT_BUILD_DIR/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/lib' -Xlinker -rpath -Xlinker '$SWIFT_BUILD_DIR/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/lib' -module-cache-path $SWIFT_BUILD_DIR/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache -I. -Xfrontend -enable-cxx-interop -o a.out -module-name main test.swift

I replaced some parts of the longer paths to cut down on a much larger command, hopefully it makes sense.
Running this line gives me the following:

error: link command failed with exit code 1 (use -v to see invocation)
Undefined symbols for architecture x86_64:
  "__Z6Answerv", referenced from:
      __ZN1AC2Ev in test-1.o
  "__ZNSt3__1L25__construct_range_forwardINS_9allocatorIiEEKiiiivEEvRT_PT0_S7_RPT1_", referenced from:
      __ZNSt3__16vectorIiNS_9allocatorIiEEE18__construct_at_endIPKiEENS_9enable_ifIXsr27__is_cpp17_forward_iteratorIT_EE5valueEvE4typeES8_S8_m in test-1.o
  "__ZNSt3__1L7forwardINS_18__default_init_tagEEEOT_RNS_16remove_referenceIS2_E4typeE", referenced from:
      __ZNSt3__117__compressed_pairIPiNS_9allocatorIiEEEC2IDnNS_18__default_init_tagEEEOT_OT0_ in test-1.o
ld: symbol(s) not found for architecture x86_64

Let me know if there's anything else you'd like to know.