mwyman updated this revision to Diff 441270.
mwyman added a comment.

Change to use a unit test on the semantic lookup code, rather than a codegen 
test; I realized the name lookup behavior could be more directly checked by 
inspecting the internal references in the generated AST to ensure the found 
"self" is the implicit parameter "self".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128556/new/

https://reviews.llvm.org/D128556

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/unittests/Sema/SemaLookupTest.cpp

Index: clang/unittests/Sema/SemaLookupTest.cpp
===================================================================
--- clang/unittests/Sema/SemaLookupTest.cpp
+++ clang/unittests/Sema/SemaLookupTest.cpp
@@ -1,4 +1,6 @@
 #include "clang/AST/DeclarationName.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Parse/ParseAST.h"
@@ -10,6 +12,7 @@
 using namespace llvm;
 using namespace clang;
 using namespace clang::tooling;
+using namespace clang::ast_matchers;
 
 namespace {
 
@@ -57,4 +60,84 @@
                                     file_contents, {"-x", "objective-c++"},
                                     "test.mm"));
 }
+
+using ExprMatcher = internal::Matcher<Expr>;
+using StmtMatcher = internal::Matcher<Stmt>;
+
+AST_MATCHER(ObjCIvarRefExpr, isFreeIvar) {
+  return Node.isFreeIvar();
+}
+
+ExprMatcher objcIvarRefTo(StringRef Name) {
+  return declRefExpr(hasAncestor(objcIvarRefExpr(allOf(isFreeIvar(),
+                                                       hasDeclaration(namedDecl(
+                                                           hasName(Name)))))
+                                     .bind("ivarref")))
+      .bind("selfref");
+}
+
+StmtMatcher withEnclosingMethod(ExprMatcher Matcher) {
+  return expr(Matcher,
+              hasAncestor(objcMethodDecl(isDefinition()).bind("method")))
+      .bind("expr");
+}
+
+class IvarLookupImplicitSelf : public ASTFrontendAction {
+  std::unique_ptr<ASTConsumer>
+  CreateASTConsumer(CompilerInstance &CI, StringRef /* Unused */) override {
+    return std::make_unique<clang::ASTConsumer>();
+  }
+
+  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();
+    const auto Results =
+        match(withEnclosingMethod(objcIvarRefTo("_ivar")), Ctx);
+    const auto *MD = selectFirst<ObjCMethodDecl>("method", Results);
+    const auto *SelfRef = selectFirst<DeclRefExpr>("selfref", Results);
+    ASSERT_TRUE(MD && SelfRef);
+
+    // When name lookup is occurring correctly, the SelfRef should refer to the
+    // Method's ImplicitParamDecl of 'self'.
+    const auto *SelfDecl = SelfRef->getDecl();
+    EXPECT_TRUE(isa<ImplicitParamDecl>(SelfDecl) &&
+                MD->getSelfDecl() == SelfDecl);
+  }
+};
+
+constexpr StringRef ImplicitIvarFileContents = R"objcxx(
+__attribute__((objc_root_class))
+@interface Foo
+@end
+extern Foo *CreateFoo();
+@implementation Foo {
+  int _ivar;
+}
+- (void)foo {
+  {
+    Foo *self = CreateFoo();
+    _ivar = 42;
+  }
+}
+@end
+  )objcxx";
+
+TEST(SemaLookupTest, ImplicitIvarSelfRefLookupObjCpp) {
+  ASSERT_TRUE(runToolOnCodeWithArgs(std::make_unique<IvarLookupImplicitSelf>(),
+                                    ImplicitIvarFileContents,
+                                    {"-x", "objective-c++"}, "test.mm"));
+}
+
+TEST(SemaLookupTest, ImplicitIvarSelfRefLookupObjC) {
+  ASSERT_TRUE(runToolOnCodeWithArgs(std::make_unique<IvarLookupImplicitSelf>(),
+                                    ImplicitIvarFileContents,
+                                    {"-x", "objective-c"}, "test.m"));
+}
+
 } // namespace
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1313,6 +1313,9 @@
             R.setShadowed();
             continue;
           }
+        } else if (NameKind == LookupObjCImplicitSelfParam &&
+                   !isa<ImplicitParamDecl>(*I)) {
+          continue;
         } else {
           // We found something in this scope, we should not look at the
           // namespace scope
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to