[NFC] Add UsedDeclVisitor

[yaxunl]

Summary

This patch is extracted from https://reviews.llvm.org/D70172

[rjmccall]

Thanks, looks good except for one oversight.

clang/lib/Sema/UsedDeclVisitor.h

43

You need to recurse on the base expression here. (And that's a good test case for your own patch!)

[yaxunl]

visit base expr of member expr.

clang/lib/Sema/UsedDeclVisitor.h

43

fixed

[rjmccall]

Thanks, LGTM.

[jpienaar]

This does not appear to be NFC:
git checkout 704cd4d5d0754904361823588f203369c309deca ; ninja check-mlir passes
git checkout 08ab8c9af4dd27cb306b449edc9a9c50ed11194a ; ninja check-mlir fails with:

0.      Program arguments: Compiles/build_clang/bin/clang++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/mlir/lib/Transforms -Illvm-project/mlir/lib/Transforms -Iinclude -Illvm-project/llvm/include -Illvm-project/mlir/include -Itools/mlir/include -fPIC -fvisibility-inl
ines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o -MF tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o.d -o tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o -c llvm-project/mlir/lib/Transforms/AffineDataCopyGeneration.cpp
1.      <eof> parser at end of file
2.      Per-file LLVM IR generation
3.      /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:848:5: Generating code for declaration 'std::make_unique'
build_clang/bin/clang++(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x55796bf8436a]
build_clang/bin/clang++(_ZN4llvm3sys17RunSignalHandlersEv+0x34)[0x55796bf82204]
build_clang/bin/clang++(_ZN4llvm3sys15CleanupOnSignalEm+0xf8)[0x55796bf82708]
build_clang/bin/clang++(+0x1a58a08)[0x55796bf07a08]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x13520)[0x7f1b727f7520]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x121)[0x7f1b722bd535]
/lib/x86_64-linux-gnu/libc.so.6(+0x2540f)[0x7f1b722bd40f]
/lib/x86_64-linux-gnu/libc.so.6(+0x32b92)[0x7f1b722cab92]
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction17EmitDeclRefLValueEPKNS_11DeclRefExprE+0x358)[0x55796c4cb138]
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction10EmitLValueEPKNS_4ExprE+0x2f9)[0x55796c4ca6d9]
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction14EmitCastLValueEPKNS_8CastExprE+0x103)[0x55796c4d4033]

[yaxunl]

In D76262#1929918, @jpienaar wrote:

This does not appear to be NFC:
git checkout 704cd4d5d0754904361823588f203369c309deca ; ninja check-mlir passes
git checkout 08ab8c9af4dd27cb306b449edc9a9c50ed11194a ; ninja check-mlir fails with:

0.      Program arguments: Compiles/build_clang/bin/clang++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/mlir/lib/Transforms -Illvm-project/mlir/lib/Transforms -Iinclude -Illvm-project/llvm/include -Illvm-project/mlir/include -Itools/mlir/include -fPIC -fvisibility-inl
ines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o -MF tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o.d -o tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o -c llvm-project/mlir/lib/Transforms/AffineDataCopyGeneration.cpp
1.      <eof> parser at end of file
2.      Per-file LLVM IR generation
3.      /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:848:5: Generating code for declaration 'std::make_unique'
build_clang/bin/clang++(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x55796bf8436a]
build_clang/bin/clang++(_ZN4llvm3sys17RunSignalHandlersEv+0x34)[0x55796bf82204]
build_clang/bin/clang++(_ZN4llvm3sys15CleanupOnSignalEm+0xf8)[0x55796bf82708]
build_clang/bin/clang++(+0x1a58a08)[0x55796bf07a08]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x13520)[0x7f1b727f7520]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x121)[0x7f1b722bd535]
/lib/x86_64-linux-gnu/libc.so.6(+0x2540f)[0x7f1b722bd40f]
/lib/x86_64-linux-gnu/libc.so.6(+0x32b92)[0x7f1b722cab92]
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction17EmitDeclRefLValueEPKNS_11DeclRefExprE+0x358)[0x55796c4cb138]
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction10EmitLValueEPKNS_4ExprE+0x2f9)[0x55796c4ca6d9]
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction14EmitCastLValueEPKNS_8CastExprE+0x103)[0x55796c4d4033]

Fixed by 62201763c54166a3ee5552975cc3fc06db2d5c6f

[thakis]

Could you add a clang-side test for the check-mlir fix? Having to run mlir tests to verify clang changes seems suboptimal, so we should try to improve coverage of clang's tests.

[yaxunl]

In D76262#1930297, @thakis wrote:

Could you add a clang-side test for the check-mlir fix? Having to run mlir tests to verify clang changes seems suboptimal, so we should try to improve coverage of clang's tests.

sure. will do.

[yaxunl]

In D76262#1930297, @thakis wrote:

Could you add a clang-side test for the check-mlir fix? Having to run mlir tests to verify clang changes seems suboptimal, so we should try to improve coverage of clang's tests.

test added by b58f6bb1207521cdf6a856004525d7bffa5f2153

clang/lib/Sema/SemaExpr.cpp

 //===--- SemaExpr.cpp - Semantic Analysis for Expressions -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements semantic analysis for expressions.
 //
 //===----------------------------------------------------------------------===//
 
 #include "TreeTransform.h"
+#include "UsedDeclVisitor.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"

 }
 
 void Sema::MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T) {
   MarkReferencedDecls Marker(*this, Loc);
   Marker.TraverseType(T);
 }
 
 namespace {
-  /// Helper class that marks all of the declarations referenced by
-  /// potentially-evaluated subexpressions as "referenced".
-  class EvaluatedExprMarker : public EvaluatedExprVisitor<EvaluatedExprMarker> {
-    Sema &S;
-    bool SkipLocalVariables;
-
-  public:
-    typedef EvaluatedExprVisitor<EvaluatedExprMarker> Inherited;
+class EvaluatedExprMarker : public UsedDeclVisitor<EvaluatedExprMarker> {
+  typedef UsedDeclVisitor<EvaluatedExprMarker> Inherited;
+  bool SkipLocalVariables;
 
-    EvaluatedExprMarker(Sema &S, bool SkipLocalVariables)
-      : Inherited(S.Context), S(S), SkipLocalVariables(SkipLocalVariables) { }
+      : Inherited(S), SkipLocalVariables(SkipLocalVariables) {}
+
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+    S.MarkFunctionReferenced(Loc, cast<FunctionDecl>(D));
+  }
 
-    void VisitDeclRefExpr(DeclRefExpr *E) {
-      // If we were asked not to visit local variables, don't.
-      if (SkipLocalVariables) {
-        if (VarDecl *VD = dyn_cast<VarDecl>(E->getDecl()))
-          if (VD->hasLocalStorage())
-            return;
-      }
-
-      S.MarkDeclRefReferenced(E);
-    }
 
-    void VisitMemberExpr(MemberExpr *E) {
-      S.MarkMemberReferenced(E);
-      Inherited::VisitMemberExpr(E);
-    }
-
-    void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
-      S.MarkFunctionReferenced(
-          E->getBeginLoc(),
-          const_cast<CXXDestructorDecl *>(E->getTemporary()->getDestructor()));
-      Visit(E->getSubExpr());
-    }
-
-    void VisitCXXNewExpr(CXXNewExpr *E) {
-      if (E->getOperatorNew())
-        S.MarkFunctionReferenced(E->getBeginLoc(), E->getOperatorNew());
-      if (E->getOperatorDelete())
-        S.MarkFunctionReferenced(E->getBeginLoc(), E->getOperatorDelete());
-      Inherited::VisitCXXNewExpr(E);
-    }
-
-    void VisitCXXDeleteExpr(CXXDeleteExpr *E) {
-      if (E->getOperatorDelete())
-        S.MarkFunctionReferenced(E->getBeginLoc(), E->getOperatorDelete());
-      QualType Destroyed = S.Context.getBaseElementType(E->getDestroyedType());
-      if (const RecordType *DestroyedRec = Destroyed->getAs<RecordType>()) {
-        CXXRecordDecl *Record = cast<CXXRecordDecl>(DestroyedRec->getDecl());
-        S.MarkFunctionReferenced(E->getBeginLoc(), S.LookupDestructor(Record));
-      }
-
-      Inherited::VisitCXXDeleteExpr(E);
-    }
-
-    void VisitCXXConstructExpr(CXXConstructExpr *E) {
-      S.MarkFunctionReferenced(E->getBeginLoc(), E->getConstructor());
-      Inherited::VisitCXXConstructExpr(E);
-    }
-
-    void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
-      Visit(E->getExpr());
-    }
-  };
-}
+  void VisitMemberExpr(MemberExpr *E) { S.MarkMemberReferenced(E); }
+} // namespace
 
 /// Mark any declarations that appear within this expression or any
 /// potentially-evaluated subexpressions as "referenced".
 ///
 /// \param SkipLocalVariables If true, don't mark local variables as
 /// 'referenced'.
 void Sema::MarkDeclarationsReferencedInExpr(Expr *E,
                                             bool SkipLocalVariables) {

clang/lib/Sema/UsedDeclVisitor.h

+//===- UsedDeclVisitor.h - ODR-used declarations visitor --------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//===----------------------------------------------------------------------===//
+//
+//  This file defines UsedDeclVisitor, a CRTP class which visits all the
+//  declarations that are ODR-used by an expression or statement.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+#define LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+
+#include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/Sema/SemaInternal.h"
+
+namespace clang {
+template <class Derived>
+class UsedDeclVisitor : public EvaluatedExprVisitor<Derived> {
+protected:
+  Sema &S;
+
+public:
+  typedef EvaluatedExprVisitor<Derived> Inherited;
+
+  UsedDeclVisitor(Sema &S) : Inherited(S.Context), S(S) {}
+
+  Derived &asImpl() { return *static_cast<Derived *>(this); }
+
+  void VisitDeclRefExpr(DeclRefExpr *E) {
+    auto *D = E->getDecl();
+    if (isa<FunctionDecl>(D) || isa<VarDecl>(D)) {
+      asImpl().visitUsedDecl(E->getLocation(), D);
+    }
+  }
+
+  void VisitMemberExpr(MemberExpr *E) {
+    auto *D = E->getMemberDecl();
+    if (isa<FunctionDecl>(D) || isa<VarDecl>(D)) {
+      asImpl().visitUsedDecl(E->getMemberLoc(), D);
+    }
+  }
+
+  void VisitCapturedStmt(CapturedStmt *Node) {
+    asImpl().visitUsedDecl(Node->getBeginLoc(), Node->getCapturedDecl());
+    Inherited::VisitCapturedStmt(Node);
+  }
+
+  void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
+    asImpl().visitUsedDecl(
+        E->getBeginLoc(),
+        const_cast<CXXDestructorDecl *>(E->getTemporary()->getDestructor()));
+    asImpl().Visit(E->getSubExpr());
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+    if (E->getOperatorNew())
+      asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorNew());
+    if (E->getOperatorDelete())
+      asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+    Inherited::VisitCXXNewExpr(E);
+  }
+
+  void VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+    if (E->getOperatorDelete())
+      asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+    QualType Destroyed = S.Context.getBaseElementType(E->getDestroyedType());
+    if (const RecordType *DestroyedRec = Destroyed->getAs<RecordType>()) {
+      CXXRecordDecl *Record = cast<CXXRecordDecl>(DestroyedRec->getDecl());
+      asImpl().visitUsedDecl(E->getBeginLoc(), S.LookupDestructor(Record));
+    }
+
+    Inherited::VisitCXXDeleteExpr(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+    asImpl().visitUsedDecl(E->getBeginLoc(), E->getConstructor());
+    Inherited::VisitCXXConstructExpr(E);
+  }
+
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+    asImpl().Visit(E->getExpr());
+  }
+};
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H