Page MenuHomePhabricator

[clang][Sema] Add flag to LookupName to force C/ObjC codepath
ClosedPublic

Authored by bulbazord on Mar 29 2022, 3:52 PM.

Details

Summary

Motivation: The intent here is for use in Swift.
When building a clang module for swift consumption, swift adds an
extension block to the module for name lookup purposes. Swift calls
this a SwiftLookupTable. One purpose that this serves is to handle
conflicting names between ObjC classes and ObjC protocols. They exist in
different namespaces in ObjC programs, but in Swift they would exist in
the same namespace. Swift handles this by appending a suffix to a
protocol name if it shares a name with a class. For example, if you have
an ObjC class named "Foo" and a protocol with the same name, the
protocol would be renamed to "FooProtocol" when imported into swift.

When constructing the previously mentioned SwiftLookupTable, we use
Sema::LookupName to look up name conflicts for the previous problem.
By this time, the Parser has long finished its job so the call to
LookupName gets nullptr for its Scope (TUScope will be nullptr
by this point). The C/ObjC path does not have this problem because it
only uses the Scope in specific scenarios. The C++ codepath uses the
Scope quite extensively and will fail early on if the Scope it gets is
null. In our very specific case of looking up ObjC classes with a
specific name, we want to force sema::LookupName to take the C/ObjC
codepath even if C++ or ObjC++ is enabled.

Diff Detail

Event Timeline

bulbazord created this revision.Mar 29 2022, 3:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2022, 3:53 PM
bulbazord requested review of this revision.Mar 29 2022, 3:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2022, 3:53 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

@rjmccall would you be able to review this patch? What do you think of this approach? This change is to support C++-Interop on the Swift side.

The change itself LGTM.

It would be really good if we can have a unit test that tests the new API when it lands, to ensure that this is tested in LLVM-project itself , in addition to Swift. Do you think it's possible to write a unit tests that constructs a compiler instance, parses a sample C++ file, and then you can use the Sema to try to invoke this LookupName API to perform the lookup to verify that it works as expected? I think it should be possible to construct such a unit test.

bulbazord edited the summary of this revision. (Show Details)

Added a test

arphaman accepted this revision.Apr 18 2022, 4:12 PM

Great, thanks for adding the test case!

This revision is now accepted and ready to land.Apr 18 2022, 4:12 PM
bulbazord closed this revision.Apr 19 2022, 1:15 PM

Gah, forgot to add the phabricator link to the commit message. I added the commit object to this diff, hopefully people can track it down the review if needed.