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