[cxx-interop] Add members to the LookupTable where possible. #39664

zoecarver · 2021-10-08T23:11:20Z

If possible, add imported members to the StructDecl's LookupTable rather than adding them directly as members. This will fix the issues with ordering that #39436 poorly attempted to solve during IRGen.

This also allows us to break out most of the test changes from #39436.

zoecarver · 2021-10-08T23:11:48Z

@swift-ci please smoke test.

egorzhdan

I think this approach looks good! However, there's an assertion that gets triggered when the Swift code has an extension of C++ struct, for example:

import std

extension std.__1.string {
  public init(_ string: String) {
    self.init()
  }
}

Stacktrace

Assertion failed: (prevEnd.isValid() && "Only implicit decls can have invalid source location"), function operator(), file /Users/egorzh/swift/swift/lib/AST/DeclContext.cpp, line 880.
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project and the crash backtrace.
Stack dump:
0.	Program arguments: /Users/egorzh/swift/swift/cmake-build-debug/bin/swift-frontend -interpret ../test/Interop/Cxx/MyTest.swift -enable-cxx-interop -sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -tools-directory /Users/egorzh/swift/build/Ninja-DebugAssert/llvm-macosx-x86_64/bin -module-cache-path /Users/egorzh/swift/build/manual-module-cache -Xcc -std=c++14 -validate-tbd-against-ir=none -target x86_64-apple-macosx10.15
1.	Swift version 5.6-dev (LLVM ae102eaadf2d38c, Swift ab8934238b1a8c9)
2.	Compiling with the current language version
3.	While evaluating request ASTLoweringRequest(Lowering AST to SIL for module MyTest)
4.	While evaluating request StoredPropertiesRequest(std_config.(file).__1 extension.__CxxTemplateInstNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE)
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  swift-frontend           0x000000011472312d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 61
1  swift-frontend           0x000000011472366b PrintStackTraceSignalHandler(void*) + 27
2  swift-frontend           0x00000001147214f3 llvm::sys::RunSignalHandlers() + 115
3  swift-frontend           0x0000000114725704 SignalHandler(int) + 196
4  libsystem_platform.dylib 0x00007fff204e2d7d _sigtramp + 29
5  libsystem_platform.dylib 000000000000000000 _sigtramp + 18446603339974169248
6  libsystem_c.dylib        0x00007fff203f2406 abort + 125
7  libsystem_c.dylib        0x00007fff203f17d8 err + 0
8  swift-frontend           0x000000010bd6b439 swift::IterableDeclContext::addMemberSilently(swift::Decl*, swift::Decl*, bool) const::$_1::operator()(swift::Decl*, swift::Decl*) const + 233
9  swift-frontend           0x000000010bd6b215 swift::IterableDeclContext::addMemberSilently(swift::Decl*, swift::Decl*, bool) const + 485
10 swift-frontend           0x000000010bd6aeaa swift::IterableDeclContext::addMember(swift::Decl*, swift::Decl*, bool) + 58
11 swift-frontend           0x000000010b5d59be loadAllMembersOfRecordDecl(swift::StructDecl*) + 878
12 swift-frontend           0x000000010b5d5221 swift::ClangImporter::Implementation::loadAllMembers(swift::Decl*, unsigned long long) + 337
13 swift-frontend           0x000000010bd6a68e swift::IterableDeclContext::loadAllMembers() const + 318
14 swift-frontend           0x000000010bd6a529 swift::IterableDeclContext::getMembers() const + 25
15 swift-frontend           0x000000010b32cc0a swift::StoredPropertiesRequest::evaluate(swift::Evaluator&, swift::NominalTypeDecl*) const + 154
16 swift-frontend           0x000000010b2f45a2 llvm::ArrayRef<swift::VarDecl*> swift::SimpleRequest<swift::StoredPropertiesRequest, llvm::ArrayRef<swift::VarDecl*> (swift::NominalTypeDecl*), (swift::RequestFlags)2>::callDerived<0ul>(swift::Evaluator&, std::__1::integer_sequence<unsigned long, 0ul>) const + 66
17 swift-frontend           0x000000010b2f026d swift::SimpleRequest<swift::StoredPropertiesRequest, llvm::ArrayRef<swift::VarDecl*> (swift::NominalTypeDecl*), (swift::RequestFlags)2>::evaluateRequest(swift::StoredPropertiesRequest const&, swift::Evaluator&) + 29
18 swift-frontend           0x000000010bb6ae55 llvm::Expected<swift::StoredPropertiesRequest::OutputType> swift::Evaluator::getResultUncached<swift::StoredPropertiesRequest>(swift::StoredPropertiesRequest const&) + 309
19 swift-frontend           0x000000010bb6ac3a llvm::Expected<swift::StoredPropertiesRequest::OutputType> swift::Evaluator::getResultCached<swift::StoredPropertiesRequest, (void*)0>(swift::StoredPropertiesRequest const&) + 250
20 swift-frontend           0x000000010bb6a97e llvm::Expected<swift::StoredPropertiesRequest::OutputType> swift::Evaluator::operator()<swift::StoredPropertiesRequest, (void*)0>(swift::StoredPropertiesRequest const&) + 78
21 swift-frontend           0x000000010ba7feda swift::StoredPropertiesRequest::OutputType swift::evaluateOrDefault<swift::StoredPropertiesRequest>(swift::Evaluator&, swift::StoredPropertiesRequest, swift::StoredPropertiesRequest::OutputType) + 58
22 swift-frontend           0x000000010ba7fe8a swift::NominalTypeDecl::getStoredProperties() const + 106
23 swift-frontend           0x00000001090f1fe0 (anonymous namespace)::LowerType::visitAnyStructType(swift::CanType, swift::Lowering::AbstractionPattern, swift::StructDecl*, swift::Lowering::IsTypeExpansionSensitive_t) + 400
24 swift-frontend           0x00000001090e6c39 (anonymous namespace)::TypeClassifierBase<(anonymous namespace)::LowerType, swift::Lowering::TypeLowering*>::visitStructType(swift::CanTypeWrapper<swift::StructType>, swift::Lowering::AbstractionPattern, swift::Lowering::IsTypeExpansionSensitive_t) + 169
25 swift-frontend           0x00000001090c841b swift::CanTypeVisitor<(anonymous namespace)::LowerType, swift::Lowering::TypeLowering*, swift::Lowering::AbstractionPattern, swift::Lowering::IsTypeExpansionSensitive_t>::visit(swift::CanType, swift::Lowering::AbstractionPattern, swift::Lowering::IsTypeExpansionSensitive_t) + 3931
26 swift-frontend           0x00000001090c7046 swift::Lowering::TypeConverter::getTypeLowering(swift::Lowering::AbstractionPattern, swift::Type, swift::TypeExpansionContext) + 1142
27 swift-frontend           0x0000000108f5fbe2 (anonymous namespace)::DestructureResults::destructure(swift::Lowering::AbstractionPattern, swift::CanType) + 706
28 swift-frontend           0x0000000108f5e5ac getSILFunctionType(swift::Lowering::TypeConverter&, swift::TypeExpansionContext, swift::Lowering::AbstractionPattern, swift::CanTypeWrapper<swift::AnyFunctionType>, swift::SILExtInfoBuilder, (anonymous namespace)::Conventions const&, swift::ForeignInfo const&, llvm::Optional<swift::SILDeclRef>, llvm::Optional<swift::SILDeclRef>, llvm::Optional<swift::SubstitutionMap>, swift::ProtocolConformanceRef) + 2764
29 swift-frontend           0x0000000108f5d8e1 getNativeSILFunctionType(swift::Lowering::TypeConverter&, swift::TypeExpansionContext, swift::Lowering::AbstractionPattern, swift::CanTypeWrapper<swift::AnyFunctionType>, swift::SILExtInfoBuilder, llvm::Optional<swift::SILDeclRef>, llvm::Optional<swift::SILDeclRef>, llvm::Optional<swift::SubstitutionMap>, swift::ProtocolConformanceRef)::$_11::operator()((anonymous namespace)::Conventions const&) const + 545
30 swift-frontend           0x0000000108f37fcb getNativeSILFunctionType(swift::Lowering::TypeConverter&, swift::TypeExpansionContext, swift::Lowering::AbstractionPattern, swift::CanTypeWrapper<swift::AnyFunctionType>, swift::SILExtInfoBuilder, llvm::Optional<swift::SILDeclRef>, llvm::Optional<swift::SILDeclRef>, llvm::Optional<swift::SubstitutionMap>, swift::ProtocolConformanceRef) + 875
31 swift-frontend           0x0000000108f3963b getUncachedSILFunctionTypeForConstant(swift::Lowering::TypeConverter&, swift::TypeExpansionContext, swift::SILDeclRef, swift::Lowering::TypeConverter::LoweredFormalTypes) + 1227
32 swift-frontend           0x0000000108f3a010 swift::Lowering::TypeConverter::getConstantInfo(swift::TypeExpansionContext, swift::SILDeclRef) + 672
33 swift-frontend           0x0000000109bd1556 swift::Lowering::TypeConverter::getConstantFunctionType(swift::TypeExpansionContext, swift::SILDeclRef) + 134
34 swift-frontend           0x0000000108f2d376 swift::SILFunctionBuilder::getOrCreateFunction(swift::SILLocation, swift::SILDeclRef, swift::ForDefinition_t, llvm::function_ref<swift::SILFunction* (swift::SILLocation, swift::SILDeclRef)>, swift::ProfileCounter) + 246
35 swift-frontend           0x0000000109bc05d9 swift::SILFunction* swift::Lowering::SILGenFunctionBuilder::getOrCreateFunction<swift::Decl*, swift::SILDeclRef&, swift::ForDefinition_t&, swift::Lowering::SILGenModule::getFunction(swift::SILDeclRef, swift::ForDefinition_t)::$_0>(swift::Decl*&&, swift::SILDeclRef&, swift::ForDefinition_t&, swift::Lowering::SILGenModule::getFunction(swift::SILDeclRef, swift::ForDefinition_t)::$_0&&) + 217
36 swift-frontend           0x0000000109bbfe1c swift::Lowering::SILGenModule::getFunction(swift::SILDeclRef, swift::ForDefinition_t) + 364
37 swift-frontend           0x0000000109bc7256 emitOrDelayFunction(swift::Lowering::SILGenModule&, swift::SILDeclRef, bool) + 518
38 swift-frontend           0x0000000109bc7639 swift::Lowering::SILGenModule::emitConstructor(swift::ConstructorDecl*) + 713
39 swift-frontend           0x0000000109e1c3f4 SILGenExtension::visitConstructorDecl(swift::ConstructorDecl*) + 36
40 swift-frontend           0x0000000109e1bd4b swift::ASTVisitor<SILGenExtension, void, void, void, void, void, void>::visit(swift::Decl*) + 891
41 swift-frontend           0x0000000109e0d57c SILGenExtension::emitExtension(swift::ExtensionDecl*) + 140
42 swift-frontend           0x0000000109e0d4ba swift::Lowering::SILGenModule::visitExtensionDecl(swift::ExtensionDecl*) + 42
43 swift-frontend           0x0000000109bdbfec swift::ASTVisitor<swift::Lowering::SILGenModule, void, void, void, void, void, void>::visit(swift::Decl*) + 1356
44 swift-frontend           0x0000000109bcb7c5 (anonymous namespace)::SILGenModuleRAII::emitSourceFile(swift::SourceFile*) + 229
45 swift-frontend           0x0000000109bcb3c3 swift::ASTLoweringRequest::evaluate(swift::Evaluator&, swift::ASTLoweringDescriptor) const + 883
46 swift-frontend           0x0000000109de7082 std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> > swift::SimpleRequest<swift::ASTLoweringRequest, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> > (swift::ASTLoweringDescriptor), (swift::RequestFlags)9>::callDerived<0ul>(swift::Evaluator&, std::__1::integer_sequence<unsigned long, 0ul>) const + 130
47 swift-frontend           0x0000000109de6fab swift::SimpleRequest<swift::ASTLoweringRequest, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> > (swift::ASTLoweringDescriptor), (swift::RequestFlags)9>::evaluateRequest(swift::ASTLoweringRequest const&, swift::Evaluator&) + 43
48 swift-frontend           0x0000000109bf08bc llvm::Expected<swift::ASTLoweringRequest::OutputType> swift::Evaluator::getResultUncached<swift::ASTLoweringRequest>(swift::ASTLoweringRequest const&) + 316
49 swift-frontend           0x0000000109bcbdab llvm::Expected<swift::ASTLoweringRequest::OutputType> swift::Evaluator::operator()<swift::ASTLoweringRequest, (void*)0>(swift::ASTLoweringRequest const&) + 43
50 swift-frontend           0x0000000109bcbcae swift::performASTLowering(swift::ModuleDecl*, swift::Lowering::TypeConverter&, swift::SILOptions const&) + 238
51 swift-frontend           0x0000000108951e2b performCompileStepsPostSema(swift::CompilerInstance&, int&, swift::FrontendObserver*) + 187
52 swift-frontend           0x0000000108951d63 performAction(swift::CompilerInstance&, int&, swift::FrontendObserver*)::$_18::operator()(swift::CompilerInstance&) const + 147
53 swift-frontend           0x0000000108951cbd bool llvm::function_ref<bool (swift::CompilerInstance&)>::callback_fn<performAction(swift::CompilerInstance&, int&, swift::FrontendObserver*)::$_18>(long, swift::CompilerInstance&) + 45
54 swift-frontend           0x0000000108948259 llvm::function_ref<bool (swift::CompilerInstance&)>::operator()(swift::CompilerInstance&) const + 57
55 swift-frontend           0x0000000108946a6e withSemanticAnalysis(swift::CompilerInstance&, swift::FrontendObserver*, llvm::function_ref<bool (swift::CompilerInstance&)>) + 350
56 swift-frontend           0x000000010894003a performAction(swift::CompilerInstance&, int&, swift::FrontendObserver*) + 1018
57 swift-frontend           0x0000000108932ba0 performCompile(swift::CompilerInstance&, int&, swift::FrontendObserver*) + 288
58 swift-frontend           0x0000000108931ad6 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 2534
59 swift-frontend           0x0000000108897134 run_driver(llvm::StringRef, llvm::ArrayRef<char const*>) + 916
60 swift-frontend           0x00000001088962a5 swift::mainEntry(int, char const**) + 1653
61 swift-frontend           0x0000000108895a02 main + 34
62 libdyld.dylib            0x00007fff204b8f3d start + 1

zoecarver · 2021-10-11T18:18:42Z

I think this approach looks good! However, there's an assertion that gets triggered when the Swift code has an extension of C++ struct, for example:

Good catch! Fixed :)

lib/Sema/TypeCheckNameLookup.cpp

zoecarver · 2021-10-11T19:35:24Z

@swift-ci please smoke test.

zoecarver · 2021-10-12T01:37:07Z

@swift-ci please smoke test.

zoecarver · 2021-10-12T01:39:39Z

@beccadax @slavapestov friendly ping. Any chance you could review this change?

zoecarver · 2021-10-12T15:46:36Z

@swift-ci please smoke test macOS.

test/Interop/Cxx/class/linked-records-module-interface.swift

guitard0g

LGTM!

zoecarver · 2021-10-13T18:05:12Z

@swift-ci please smoke test.

zoecarver · 2021-10-13T18:06:00Z

When the test pass, I will land this PR. If you have any other review comment, please let me know!

zoecarver · 2021-10-13T18:09:50Z

Thanks, Egor :)

If possible, add imported members to the StructDecl's LookupTable rather than adding them directly as members. This will fix the issues with ordering that swiftlang#39436 poorly attempted to solve during IRGen. This also allows us to break out most of the test changes from swiftlang#39436.

zoecarver · 2021-10-13T18:54:18Z

@swift-ci please smoke test.

When swiftlang#39664 moved the logic for generating anonymous fields' names from ImportDecl to ImportName, it inadvertently replaced a check that the decl was *precisely* `clang::FieldDecl` with a check that it was `FieldDecl` *or a subclass*. This could cause a crash when it tried to call `FieldDecl::getFieldIndex()`, which doesn't work properly on instance variables even though `ObjCIvarDecl` is a subclass of `FieldDecl`. The easiest way to reproduce this is to use a bit field in a class's instance variables, since clang inserts an anonymous instance variable after it for padding. This commit adds a test of a bit field instance variable and fixes the bug. It also adds a PrettyStackTrace frame in the Swift lookup table preparation code, which should make other bugs like this easier to diagnose. Fixes rdar://85173321.

zoecarver added the c++ interop Feature: Interoperability with C++ label Oct 8, 2021

zoecarver requested review from egorzhdan, slavapestov, hamishknight, hyp, guitard0g and CodaFi October 8, 2021 23:11

egorzhdan reviewed Oct 10, 2021

View reviewed changes

zoecarver force-pushed the lazy-pt9-use-lookup-table branch 2 times, most recently from 9e13f92 to 50fd922 Compare October 11, 2021 18:18

zoecarver commented Oct 11, 2021

View reviewed changes

lib/Sema/TypeCheckNameLookup.cpp Outdated Show resolved Hide resolved

zoecarver force-pushed the lazy-pt9-use-lookup-table branch from c276008 to 5fd7185 Compare October 12, 2021 01:36

zoecarver commented Oct 12, 2021

View reviewed changes

test/Interop/Cxx/class/linked-records-module-interface.swift Outdated Show resolved Hide resolved

guitard0g approved these changes Oct 12, 2021

View reviewed changes

zoecarver force-pushed the lazy-pt9-use-lookup-table branch from 5fd7185 to a12ab5e Compare October 13, 2021 18:04

egorzhdan approved these changes Oct 13, 2021

View reviewed changes

zoecarver force-pushed the lazy-pt9-use-lookup-table branch from a12ab5e to eeeb27d Compare October 13, 2021 18:54

zoecarver merged commit 40422f0 into swiftlang:main Oct 13, 2021

beccadax mentioned this pull request Nov 10, 2021

[ClangImporter] Fix (lack of) name importing for ObjC ivars #40114

Merged

drodriguez mentioned this pull request Nov 24, 2021

[DO NOT MERGE] Test, workaround for compiler crash #40288

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[cxx-interop] Add members to the LookupTable where possible. #39664

[cxx-interop] Add members to the LookupTable where possible. #39664

Uh oh!

zoecarver commented Oct 8, 2021

Uh oh!

zoecarver commented Oct 8, 2021

Uh oh!

egorzhdan left a comment

Uh oh!

zoecarver commented Oct 11, 2021

Uh oh!

Uh oh!

zoecarver commented Oct 11, 2021

Uh oh!

zoecarver commented Oct 12, 2021

Uh oh!

zoecarver commented Oct 12, 2021

Uh oh!

zoecarver commented Oct 12, 2021

Uh oh!

Uh oh!

guitard0g left a comment

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

Uh oh!

[cxx-interop] Add members to the LookupTable where possible. #39664

[cxx-interop] Add members to the LookupTable where possible. #39664

Uh oh!

Conversation

zoecarver commented Oct 8, 2021

Uh oh!

zoecarver commented Oct 8, 2021

Uh oh!

egorzhdan left a comment

Choose a reason for hiding this comment

Uh oh!

zoecarver commented Oct 11, 2021

Uh oh!

Uh oh!

zoecarver commented Oct 11, 2021

Uh oh!

zoecarver commented Oct 12, 2021

Uh oh!

zoecarver commented Oct 12, 2021

Uh oh!

zoecarver commented Oct 12, 2021

Uh oh!

Uh oh!

guitard0g left a comment

Choose a reason for hiding this comment

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

zoecarver commented Oct 13, 2021

Uh oh!

Uh oh!