From c57f03415f9668f942802ff7108410d57c2b10da Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Mon, 18 Apr 2022 11:41:20 -0700 Subject: [PATCH] [clang][Sema] Add flag to LookupName to force C/ObjC codepath 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. --- clang/include/clang/Sema/Sema.h | 4 +- clang/lib/Sema/SemaLookup.cpp | 5 ++- clang/unittests/Sema/CMakeLists.txt | 1 + clang/unittests/Sema/SemaLookupTest.cpp | 60 +++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 clang/unittests/Sema/SemaLookupTest.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index e2ae02ea9f76..d5b73686e84a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4252,8 +4252,8 @@ public: = NotForRedeclaration); bool LookupBuiltin(LookupResult &R); void LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID); - bool LookupName(LookupResult &R, Scope *S, - bool AllowBuiltinCreation = false); + bool LookupName(LookupResult &R, Scope *S, bool AllowBuiltinCreation = false, + bool ForceNoCPlusPlus = false); bool LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx, bool InUnqualifiedLookup = false); bool LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx, diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index 65f5112afee3..ae3923854b52 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -1931,13 +1931,14 @@ NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const { /// used to diagnose ambiguities. /// /// @returns \c true if lookup succeeded and false otherwise. -bool Sema::LookupName(LookupResult &R, Scope *S, bool AllowBuiltinCreation) { +bool Sema::LookupName(LookupResult &R, Scope *S, bool AllowBuiltinCreation, + bool ForceNoCPlusPlus) { DeclarationName Name = R.getLookupName(); if (!Name) return false; LookupNameKind NameKind = R.getLookupKind(); - if (!getLangOpts().CPlusPlus) { + if (!getLangOpts().CPlusPlus || ForceNoCPlusPlus) { // Unqualified name lookup in C/Objective-C is purely lexical, so // search in the declarations attached to the name. if (NameKind == Sema::LookupRedeclarationWithLinkage) { diff --git a/clang/unittests/Sema/CMakeLists.txt b/clang/unittests/Sema/CMakeLists.txt index 194b7640b3c1..455c321d541b 100644 --- a/clang/unittests/Sema/CMakeLists.txt +++ b/clang/unittests/Sema/CMakeLists.txt @@ -7,6 +7,7 @@ add_clang_unittest(SemaTests ExternalSemaSourceTest.cpp CodeCompleteTest.cpp GslOwnerPointerInference.cpp + SemaLookupTest.cpp ) clang_target_link_libraries(SemaTests diff --git a/clang/unittests/Sema/SemaLookupTest.cpp b/clang/unittests/Sema/SemaLookupTest.cpp new file mode 100644 index 000000000000..d97b571f6a37 --- /dev/null +++ b/clang/unittests/Sema/SemaLookupTest.cpp @@ -0,0 +1,60 @@ +#include "clang/AST/DeclarationName.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/FrontendAction.h" +#include "clang/Parse/ParseAST.h" +#include "clang/Sema/Lookup.h" +#include "clang/Sema/Sema.h" +#include "clang/Tooling/Tooling.h" +#include "gtest/gtest.h" + +using namespace llvm; +using namespace clang; +using namespace clang::tooling; + +namespace { + +class LookupAction : public ASTFrontendAction { + std::unique_ptr + CreateASTConsumer(CompilerInstance &CI, StringRef /*Unused*/) override { + return std::make_unique(); + } + + void ExecuteAction() override { + CompilerInstance &CI = getCompilerInstance(); + ASSERT_FALSE(CI.hasSema()); + CI.createSema(getTranslationUnitKind(), nullptr); + ASSERT_TRUE(CI.hasSema()); + Sema &S = CI.getSema(); + ParseAST(S); + + ASTContext &Ctx = S.getASTContext(); + auto Name = &Ctx.Idents.get("Foo"); + LookupResult R_cpp(S, Name, SourceLocation(), Sema::LookupOrdinaryName); + S.LookupName(R_cpp, S.TUScope, /*AllowBuiltinCreation=*/false, + /*ForceNoCPlusPlus=*/false); + // By this point, parsing is done and S.TUScope is nullptr + // CppLookupName will perform an early return with no results if the Scope + // we pass in is nullptr. We expect to find nothing. + ASSERT_TRUE(R_cpp.empty()); + + // On the other hand, the non-C++ path doesn't care if the Scope passed in + // is nullptr. We'll force the non-C++ path with a flag. + LookupResult R_nocpp(S, Name, SourceLocation(), Sema::LookupOrdinaryName); + S.LookupName(R_nocpp, S.TUScope, /*AllowBuiltinCreation=*/false, + /*ForceNoCPlusPlus=*/true); + ASSERT_TRUE(!R_nocpp.empty()); + } +}; + +TEST(SemaLookupTest, ForceNoCPlusPlusPath) { + const char *file_contents = R"objcxx( +@protocol Foo +@end +@interface Foo +@end + )objcxx"; + ASSERT_TRUE(runToolOnCodeWithArgs(std::make_unique(), + file_contents, {"-x", "objective-c++"}, + "test.mm")); +} +} // namespace