rtrieu created this revision.
rtrieu added a subscriber: cfe-commits.
Early prototype of an improved ODR checker for Clang and modules. The current
ODR checking of classes is limited to a small number of attributes such as
number of methods and base classes. This still allows a number of ODR
violations to pass through undetected which only manifests as problems during
linking. This improved ODR checker calculates a hash based on the class
contents, then checks that the two decls have the same hash before allowing
them to be merged. The test case shows a number of ODR violations that the
current Clang checker would not have caught.
The hashing relies on three visitors for Stmt's, Decl's, and Type's. For
Stmt's, the visitor behind Stmt::Profile was refactored so that a hash could be
generated without depending on pointers. For Decl's, a new DeclVisitor was
created. The Type visitor has not been written yet. Instead, Types are
converted into a string as a stand-in.
The other area for improvement is the diagnostic message. Most have the
default message stating the class has different definitions in different
modules. New specific messages will need to be created to supplement the
default message.
http://reviews.llvm.org/D21675
Files:
include/clang/AST/DeclCXX.h
include/clang/AST/Stmt.h
lib/AST/DeclCXX.cpp
lib/AST/StmtProfile.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/Modules/Inputs/odr_hash/first.h
test/Modules/Inputs/odr_hash/module.map
test/Modules/Inputs/odr_hash/second.h
test/Modules/merge-using-decls.cpp
test/Modules/odr_hash.cpp
Index: test/Modules/odr_hash.cpp
===================================================================
--- test/Modules/odr_hash.cpp
+++ test/Modules/odr_hash.cpp
@@ -0,0 +1,191 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/odr_hash -verify %s -std=c++11
+
+#include "first.h"
+#include "second.h"
+
+namespace test1 {
+S1 s1;
+// expected-error@first.h:* {{'S1' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S2 s2;
+// expected-error@first.h:* {{'S2' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S3 s3;
+// expected-error@first.h:* {{'S3' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S4 s4;
+// expected-error@first.h:* {{'S4' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S5 s5;
+// expected-error@first.h:* {{'S5' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S6 s6;
+// expected-error@second.h:* {{'S6::Second' from module 'second' is not present in definition of 'S6' in module 'first'}}
+// expected-note@first.h:* {{definition has no member 'Second'}}
+
+S7 s7;
+// expected-error@second.h:* {{'S7::foo' from module 'second' is not present in definition of 'S7' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'foo' does not match}}
+
+S8 s8;
+// expected-error@first.h:* {{'S8' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S9 s9;
+// expected-error@first.h:* {{'S9' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S10 s10;
+// expected-error@second.h:* {{'S10::(anonymous struct)::y' from module 'second' is not present in definition of 'S10::(anonymous struct at /usr/local/google/home/rtrieu/clang/miss/llvm/tools/clang/test/Modules/Inputs/odr_hash/first.h:42:3)' in module 'first'}}
+// expected-note@first.h:* {{definition has no member 'y'}}
+
+// expected-error@first.h:* {{'S10' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S11 s11;
+// expected-error@first.h:* {{'S11' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S12 s12;
+
+S13 s13;
+// expected-error@first.h:* {{'S13' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S14 s14;
+// expected-error@second.h:* {{'S14::foo' from module 'second' is not present in definition of 'S14' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'foo' does not match}}
+
+template <typename T>
+void Test15() {
+ S15<T> s15;
+// expected-error@first.h:* {{'S15' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+}
+
+void TestS16() {
+ S16 s16;
+}
+// expected-error@first.h:* {{'S16' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S17 s17;
+// expected-error@second.h:* {{'S17::Q_type' from module 'second' is not present in definition of 'S17' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'Q_type' does not match}}
+
+S18 s18;
+// expected-error@second.h:* {{'S18::X' from module 'second' is not present in definition of 'S18' in module 'first'}}
+// expected-note@first.h:* {{definition has no member 'X'}}
+
+S19 s19;
+// expected-error@first.h:* {{'S19' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S20 s20;
+// expected-error@first.h:* {{'S20' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S21 s21;
+// expected-error@first.h:* {{'S21' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S22 s22;
+// expected-error@first.h:* {{'S22' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S23 s23;
+// expected-error@second.h:* {{'S23::a' from module 'second' is not present in definition of 'S23' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'a' does not match}}
+// expected-error@second.h:* {{'S23::b' from module 'second' is not present in definition of 'S23' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'b' does not match}}
+
+S24 s24;
+// expected-error@first.h:* {{'S24' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S25 s25;
+// expected-error@first.h:* {{'S25' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+S26 s26;
+// expected-error@first.h:* {{'S26' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+S27 s27;
+// expected-error@first.h:* {{'S27' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S28 s28;
+// expected-error@first.h:* {{'S28' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S29 s29;
+// expected-error@first.h:* {{'S29' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S30 s30;
+// expected-error@first.h:* {{'S30' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S31 s31;
+// expected-error@first.h:* {{'S31' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S32 s32;
+// expected-error@first.h:* {{'S32' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S33 s33;
+// expected-error@first.h:* {{'S33' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S34 s34;
+// expected-error@second.h:* {{'S34::operator int' from module 'second' is not present in definition of 'S34' in module 'first'}}
+// expected-note@first.h:* {{definition has no member 'operator int'}}
+
+S35 s35;
+// expected-error@first.h:* {{'S35' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S36 s36;
+// expected-error@first.h:* {{'S36' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S37 s37;
+// expected-error@first.h:* {{'S37' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S38<int> s38;
+// expected-error@first.h:* {{'S38' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S39<> s39;
+// expected-error@first.h:* {{'S39' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S40<> s40;
+// expected-error@first.h:* {{'S40' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+using ::S41;
+// expected-error@first.h:* {{'S41' has different definitions in different modules; definition in module 'first' is here}}
+// expected-note@second.h:* {{definition in module 'second' is here}}
+
+S42 s42;
+// expected-error@second.h:* {{'S42::bar' from module 'second' is not present in definition of 'S42' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'bar' does not match}}
+// expected-error@second.h:* {{'S42::foo' from module 'second' is not present in definition of 'S42' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'foo' does not match}}
+
+S43 s43;
+// expected-error@second.h:* {{'S43::x' from module 'second' is not present in definition of 'S43' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'x' does not match}}
+// expected-error@second.h:* {{'S43::y' from module 'second' is not present in definition of 'S43' in module 'first'}}
+// expected-note@first.h:* {{declaration of 'y' does not match}}
+
+}
Index: test/Modules/merge-using-decls.cpp
===================================================================
--- test/Modules/merge-using-decls.cpp
+++ test/Modules/merge-using-decls.cpp
@@ -37,6 +37,9 @@
// Here, we're instantiating the definition from 'A' and merging the definition
// from 'B' into it.
+// expected-error@b.h:* {{'D::type' from module 'B' is not present in definition of 'D<T>' in module 'A'}}
+// expected-error@b.h:* {{'D::value' from module 'B' is not present in definition of 'D<T>' in module 'A'}}
+
// expected-error@b.h:* {{'E::value' from module 'B' is not present in definition of 'E<T>' in module 'A'}}
// expected-error@b.h:* {{'E::v' from module 'B' is not present in definition of 'E<T>' in module 'A'}}
Index: test/Modules/Inputs/odr_hash/second.h
===================================================================
--- test/Modules/Inputs/odr_hash/second.h
+++ test/Modules/Inputs/odr_hash/second.h
@@ -0,0 +1,203 @@
+struct S1 {
+ private:
+};
+
+struct S2Friend1 {};
+struct S2 {
+ friend S2Friend1;
+};
+
+template<class>
+struct S3Template {};
+
+struct S3 {
+ friend S3Template<double>;
+};
+
+struct S4 {
+ static_assert(1 == 1, "Second");
+};
+
+struct S5 {
+ static_assert(2 == 2, "Message");
+};
+
+struct S6 {
+ int Second();
+};
+
+struct S7 {
+ int foo();
+};
+
+struct S8 {
+ void foo() {}
+};
+
+struct S9 {
+ void foo() { int x = 5; }
+};
+
+struct S10 {
+ struct {
+ int x;
+ int y;
+ } a;
+};
+
+struct S11 {
+ void foo() { int y = sizeof(double); }
+};
+
+struct S12 {
+ int x = sizeof(x);
+ int y = sizeof(y);
+};
+
+struct S13 {
+ template <typename B> void foo();
+};
+
+struct S14 {
+ template <typename A> void foo();
+};
+
+template <typename T>
+struct S15 : T {
+ void foo() {
+ int x = __builtin_offsetof(T, second);
+ }
+};
+
+struct S16 {
+ template <template<int = 1> class Y>
+ void foo() {
+ Y<> y;
+ }
+};
+
+struct S17 {
+ template <template <typename> class T>
+ static int foo(int a = 1);
+ template <template <typename> class T, template <typename> class U>
+ using Q_type = U<int>;
+};
+
+struct S18 {
+ enum X { X1 };
+};
+
+struct S19 {
+ enum E { X1, X2 };
+};
+
+struct S20 {
+ enum E { X1 = 5};
+};
+
+struct S21 {
+ void foo() {
+ ;
+ }
+};
+
+struct S22 {
+ void foo() {
+ label_second:
+ ;
+ }
+};
+
+struct S23 {
+ typedef char a;
+ typedef int b;
+};
+
+struct S24 {
+ int foo();
+};
+
+struct S25 {
+ int x;
+ S25() {}
+};
+
+struct S26 {
+ int x;
+ S26() : x(2) {}
+};
+
+struct S27 {
+ S27(int) {}
+ S27() {}
+};
+
+struct Base1 {
+ Base1();
+ Base1(int);
+ Base1(double);
+};
+
+struct Base2 {
+ Base2();
+ Base2(int);
+ Base2(double);
+};
+
+struct S28 : public Base2 {};
+struct S29 : virtual Base2 {};
+
+struct S30 : public Base1 {
+ S30() : Base1(1.0) {}
+};
+struct S31 : virtual Base1 {
+ S31() : Base1(1.0) {}
+};
+struct S32 : public Base2, Base1 {
+ S32() : Base2(1), Base1(1.0) {}
+};
+
+struct S33 {
+ S33() : S33(5) {}
+ S33(int) {}
+};
+
+struct S34 {
+ operator int();
+};
+
+struct S35 {
+ operator bool();
+};
+
+struct S36 {
+ int x : 4;
+};
+
+struct S37 {
+ int x;
+ mutable int y;
+};
+
+template <class Y>
+struct S38 { };
+
+template <class X = double>
+struct S39 { X x; };
+
+template <int X = 7>
+struct S40 { int x = X; };
+
+template <int> class T41b{};
+template <template<int> class T = T41b>
+struct S41 {};
+
+struct S42 {
+ void foo() {}
+ void bar() const {}
+};
+
+struct S43 {
+ int x = 1;
+ static constexpr int y = 1;
+};
Index: test/Modules/Inputs/odr_hash/module.map
===================================================================
--- test/Modules/Inputs/odr_hash/module.map
+++ test/Modules/Inputs/odr_hash/module.map
@@ -0,0 +1,6 @@
+module first {
+ header "first.h"
+}
+module second {
+ header "second.h"
+}
Index: test/Modules/Inputs/odr_hash/first.h
===================================================================
--- test/Modules/Inputs/odr_hash/first.h
+++ test/Modules/Inputs/odr_hash/first.h
@@ -0,0 +1,204 @@
+struct S1 {
+ public:
+};
+
+struct S2Friend2 {};
+struct S2 {
+ friend S2Friend2;
+};
+
+template<class>
+struct S3Template {};
+
+struct S3 {
+ friend S3Template<int>;
+};
+
+struct S4 {
+ static_assert(1 == 1, "First");
+};
+
+struct S5 {
+ static_assert(1 == 1, "Message");
+};
+
+struct S6 {
+ int First();
+};
+
+struct S7 {
+ double foo();
+};
+
+struct S8 {
+ void foo();
+};
+
+struct S9 {
+ void foo() { int y = 5; }
+};
+
+struct S10 {
+ struct {
+ int x;
+ } a;
+};
+
+struct S11 {
+ void foo() { int y = sizeof(int); }
+};
+
+struct S12 {
+ int x = sizeof(x);
+ int y = sizeof(x);
+};
+
+struct S13 {
+ template <typename A> void foo();
+};
+
+struct S14 {
+ template <typename A, typename B> void foo();
+};
+
+template <typename T>
+struct S15 : T {
+ void foo() {
+ int x = __builtin_offsetof(T, first);
+ }
+};
+
+struct S16 {
+ template <template<int = 0> class Y>
+ void foo() {
+ Y<> y;
+ }
+};
+
+struct S17 {
+ template <template <typename> class T>
+ static int foo(int a = 1);
+ template <template <typename> class T, template <typename> class U>
+ using Q_type = T<int>;
+};
+
+struct S18 {
+ enum E { X1 };
+};
+
+struct S19 {
+ enum E { X1 };
+};
+
+struct S20 {
+ enum E { X1 = 1 };
+};
+
+struct S21 {
+ void foo() {
+ label:
+ ;
+ }
+};
+
+struct S22 {
+ void foo() {
+ label_first:
+ ;
+ }
+};
+
+struct S23 {
+ typedef int a;
+ typedef char b;
+};
+
+struct S24 {
+ inline int foo();
+};
+
+struct S25 {
+ int x;
+ S25() : x(5) {}
+};
+
+struct S26 {
+ int x;
+ S26() : x(5) {}
+};
+
+struct S27 {
+ explicit S27(int) {}
+ S27() {}
+};
+
+struct Base1 {
+ Base1();
+ Base1(int);
+ Base1(double);
+};
+
+struct Base2 {
+ Base2();
+ Base2(int);
+ Base2(double);
+};
+
+struct S28 : public Base1 {};
+struct S29 : virtual Base1 {};
+
+struct S30 : public Base1 {
+ S30() : Base1(1) {}
+};
+struct S31 : virtual Base1 {
+ S31() : Base1(1) {}
+};
+struct S32 : public Base1, Base2 {
+ S32() : Base1(1), Base2(1.0) {}
+};
+
+struct S33 {
+ S33() : S33(5) {}
+ S33(int) {int a;}
+};
+
+struct S34 {
+ operator bool();
+};
+
+struct S35 {
+ explicit operator bool();
+};
+
+struct S36 {
+ int x : 3;
+};
+
+struct S37 {
+ mutable int x;
+ int y;
+};
+
+template <class X>
+struct S38 { };
+
+template <class X = int>
+struct S39 { X x; };
+
+template <int X = 5>
+struct S40 { int x = X; };
+
+template <int> class T41a{};
+template <template<int> class T = T41a>
+struct S41 {};
+
+struct S42 {
+ void foo() const {}
+ void bar() {}
+};
+
+struct S43 {
+ static constexpr int x = 1;
+ int y = 1;
+};
+
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5511,6 +5511,7 @@
Record->push_back(Data.ImplicitCopyAssignmentHasConstParam);
Record->push_back(Data.HasDeclaredCopyConstructorWithConstParam);
Record->push_back(Data.HasDeclaredCopyAssignmentWithConstParam);
+ Record->push_back(Data.ODRHash);
// IsLambda bit is already saved.
Record->push_back(Data.NumBases);
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1489,6 +1489,7 @@
Data.ImplicitCopyAssignmentHasConstParam = Record[Idx++];
Data.HasDeclaredCopyConstructorWithConstParam = Record[Idx++];
Data.HasDeclaredCopyAssignmentWithConstParam = Record[Idx++];
+ Data.ODRHash = Record[Idx++];
Data.NumBases = Record[Idx++];
if (Data.NumBases)
@@ -1619,6 +1620,7 @@
OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
MATCH_FIELD(IsLambda)
+ MATCH_FIELD(ODRHash)
#undef OR_FIELD
#undef MATCH_FIELD
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13142,8 +13142,11 @@
RD->completeDefinition();
}
- if (isa<CXXRecordDecl>(Tag))
+ if (auto *RD = dyn_cast<CXXRecordDecl>(Tag)) {
FieldCollector->FinishClass();
+ if (Context.getLangOpts().Modules)
+ RD->computeODRHash();
+ }
// Exit this scope of this tag's definition.
PopDeclContext();
Index: lib/AST/StmtProfile.cpp
===================================================================
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DeclVisitor.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
@@ -25,38 +26,42 @@
namespace {
class StmtProfiler : public ConstStmtVisitor<StmtProfiler> {
+ protected:
llvm::FoldingSetNodeID &ID;
- const ASTContext &Context;
bool Canonical;
public:
- StmtProfiler(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
- bool Canonical)
- : ID(ID), Context(Context), Canonical(Canonical) { }
+ StmtProfiler(llvm::FoldingSetNodeID &ID, bool Canonical)
+ : ID(ID), Canonical(Canonical) {}
+
+ virtual ~StmtProfiler() {}
void VisitStmt(const Stmt *S);
#define STMT(Node, Base) void Visit##Node(const Node *S);
#include "clang/AST/StmtNodes.inc"
/// \brief Visit a declaration that is referenced within an expression
/// or statement.
- void VisitDecl(const Decl *D);
+ virtual void VisitDecl(const Decl *D) = 0;
/// \brief Visit a type that is referenced within an expression or
/// statement.
- void VisitType(QualType T);
+ virtual void VisitType(QualType T) = 0;
/// \brief Visit a name that occurs within an expression or statement.
- void VisitName(DeclarationName Name);
+ virtual void VisitName(DeclarationName Name) = 0;
+
+ /// \brief Visit identifiers that are not in Decl's or Type's.
+ virtual void VisitIdentifierInfo(IdentifierInfo *II) = 0;
/// \brief Visit a nested-name-specifier that occurs within an expression
/// or statement.
- void VisitNestedNameSpecifier(NestedNameSpecifier *NNS);
+ virtual void VisitNestedNameSpecifier(NestedNameSpecifier *NNS) = 0;
/// \brief Visit a template name that occurs within an expression or
/// statement.
- void VisitTemplateName(TemplateName Name);
+ virtual void VisitTemplateName(TemplateName Name) = 0;
/// \brief Visit template arguments that occur within an expression or
/// statement.
@@ -66,6 +71,134 @@
/// \brief Visit a single template argument.
void VisitTemplateArgument(const TemplateArgument &Arg);
};
+
+ class StmtProfilerWithPointers : public StmtProfiler {
+ const ASTContext &Context;
+
+ public:
+ StmtProfilerWithPointers(llvm::FoldingSetNodeID &ID,
+ const ASTContext &Context, bool Canonical)
+ : StmtProfiler(ID, Canonical), Context(Context) {}
+ private:
+ void VisitDecl(const Decl *D) override {
+ ID.AddInteger(D ? D->getKind() : 0);
+
+ if (Canonical && D) {
+ if (const NonTypeTemplateParmDecl *NTTP =
+ dyn_cast<NonTypeTemplateParmDecl>(D)) {
+ ID.AddInteger(NTTP->getDepth());
+ ID.AddInteger(NTTP->getIndex());
+ ID.AddBoolean(NTTP->isParameterPack());
+ VisitType(NTTP->getType());
+ return;
+ }
+
+ if (const ParmVarDecl *Parm = dyn_cast<ParmVarDecl>(D)) {
+ // The Itanium C++ ABI uses the type, scope depth, and scope
+ // index of a parameter when mangling expressions that involve
+ // function parameters, so we will use the parameter's type for
+ // establishing function parameter identity. That way, our
+ // definition of "equivalent" (per C++ [temp.over.link]) is at
+ // least as strong as the definition of "equivalent" used for
+ // name mangling.
+ VisitType(Parm->getType());
+ ID.AddInteger(Parm->getFunctionScopeDepth());
+ ID.AddInteger(Parm->getFunctionScopeIndex());
+ return;
+ }
+
+ if (const TemplateTypeParmDecl *TTP =
+ dyn_cast<TemplateTypeParmDecl>(D)) {
+ ID.AddInteger(TTP->getDepth());
+ ID.AddInteger(TTP->getIndex());
+ ID.AddBoolean(TTP->isParameterPack());
+ return;
+ }
+
+ if (const TemplateTemplateParmDecl *TTP =
+ dyn_cast<TemplateTemplateParmDecl>(D)) {
+ ID.AddInteger(TTP->getDepth());
+ ID.AddInteger(TTP->getIndex());
+ ID.AddBoolean(TTP->isParameterPack());
+ return;
+ }
+ }
+
+ ID.AddPointer(D ? D->getCanonicalDecl() : nullptr);
+ }
+
+ void VisitType(QualType T) override {
+ if (Canonical)
+ T = Context.getCanonicalType(T);
+
+ ID.AddPointer(T.getAsOpaquePtr());
+ }
+
+ void VisitName(DeclarationName Name) override {
+ ID.AddPointer(Name.getAsOpaquePtr());
+ }
+
+ void VisitIdentifierInfo(IdentifierInfo *II) override {
+ ID.AddPointer(II);
+ }
+
+ void VisitNestedNameSpecifier(NestedNameSpecifier *NNS) override {
+ if (Canonical)
+ NNS = Context.getCanonicalNestedNameSpecifier(NNS);
+ ID.AddPointer(NNS);
+ }
+
+ void VisitTemplateName(TemplateName Name) override {
+ if (Canonical)
+ Name = Context.getCanonicalTemplateName(Name);
+
+ Name.Profile(ID);
+ }
+ };
+
+ class StmtProfilerWithoutPointers : public StmtProfiler {
+ public:
+ StmtProfilerWithoutPointers(llvm::FoldingSetNodeID &ID)
+ : StmtProfiler(ID, false) {}
+
+ private:
+ void VisitType(QualType T) override {
+ // TODO: String of type is used instead of a proper hash for the type.
+ // Eventually, a hash built from a type visitor should be used.
+ LangOptions options;
+ PrintingPolicy policy(options);
+ policy.AnonymousTagLocations = false;
+ ID.AddString(T.getAsString(policy));
+ }
+ void VisitName(DeclarationName Name) override {
+ if (Name.isEmpty()) {
+ ID.AddInteger(0);
+ return;
+ }
+ auto Kind = Name.getNameKind();
+ ID.AddInteger(Kind);
+ // TODO: Hash the relevant parts of DeclarationName instead of using
+ // the string.
+ SmallString<64> Buffer;
+ llvm::raw_svector_ostream StrOS(Buffer);
+ Name.print(StrOS, PrintingPolicy(LangOptions()));
+ ID.AddString(Buffer);
+ }
+ void VisitIdentifierInfo(IdentifierInfo *II) override {
+ ID.AddString(II->getName());
+ }
+ void VisitDecl(const Decl *D) override {
+ ID.AddInteger(D ? D->getKind() : 0);
+ if (!D) {
+ return;
+ }
+ if (auto *ND = dyn_cast<NamedDecl>(D)) {
+ ID.AddString(ND->getNameAsString());
+ }
+ }
+ void VisitTemplateName(TemplateName Name) override { }
+ void VisitNestedNameSpecifier(NestedNameSpecifier *NNS) override { }
+ };
}
void StmtProfiler::VisitStmt(const Stmt *S) {
@@ -775,7 +908,7 @@
break;
case OffsetOfNode::Identifier:
- ID.AddPointer(ON.getFieldName());
+ VisitIdentifierInfo(ON.getFieldName());
break;
case OffsetOfNode::Base:
@@ -1355,7 +1488,7 @@
if (S->getDestroyedTypeInfo())
VisitType(S->getDestroyedType());
else
- ID.AddPointer(S->getDestroyedTypeIdentifier());
+ VisitIdentifierInfo(S->getDestroyedTypeIdentifier());
}
void StmtProfiler::VisitOverloadExpr(const OverloadExpr *S) {
@@ -1600,77 +1733,6 @@
ID.AddBoolean(S->getBridgeKind());
}
-void StmtProfiler::VisitDecl(const Decl *D) {
- ID.AddInteger(D? D->getKind() : 0);
-
- if (Canonical && D) {
- if (const NonTypeTemplateParmDecl *NTTP =
- dyn_cast<NonTypeTemplateParmDecl>(D)) {
- ID.AddInteger(NTTP->getDepth());
- ID.AddInteger(NTTP->getIndex());
- ID.AddBoolean(NTTP->isParameterPack());
- VisitType(NTTP->getType());
- return;
- }
-
- if (const ParmVarDecl *Parm = dyn_cast<ParmVarDecl>(D)) {
- // The Itanium C++ ABI uses the type, scope depth, and scope
- // index of a parameter when mangling expressions that involve
- // function parameters, so we will use the parameter's type for
- // establishing function parameter identity. That way, our
- // definition of "equivalent" (per C++ [temp.over.link]) is at
- // least as strong as the definition of "equivalent" used for
- // name mangling.
- VisitType(Parm->getType());
- ID.AddInteger(Parm->getFunctionScopeDepth());
- ID.AddInteger(Parm->getFunctionScopeIndex());
- return;
- }
-
- if (const TemplateTypeParmDecl *TTP =
- dyn_cast<TemplateTypeParmDecl>(D)) {
- ID.AddInteger(TTP->getDepth());
- ID.AddInteger(TTP->getIndex());
- ID.AddBoolean(TTP->isParameterPack());
- return;
- }
-
- if (const TemplateTemplateParmDecl *TTP =
- dyn_cast<TemplateTemplateParmDecl>(D)) {
- ID.AddInteger(TTP->getDepth());
- ID.AddInteger(TTP->getIndex());
- ID.AddBoolean(TTP->isParameterPack());
- return;
- }
- }
-
- ID.AddPointer(D? D->getCanonicalDecl() : nullptr);
-}
-
-void StmtProfiler::VisitType(QualType T) {
- if (Canonical)
- T = Context.getCanonicalType(T);
-
- ID.AddPointer(T.getAsOpaquePtr());
-}
-
-void StmtProfiler::VisitName(DeclarationName Name) {
- ID.AddPointer(Name.getAsOpaquePtr());
-}
-
-void StmtProfiler::VisitNestedNameSpecifier(NestedNameSpecifier *NNS) {
- if (Canonical)
- NNS = Context.getCanonicalNestedNameSpecifier(NNS);
- ID.AddPointer(NNS);
-}
-
-void StmtProfiler::VisitTemplateName(TemplateName Name) {
- if (Canonical)
- Name = Context.getCanonicalTemplateName(Name);
-
- Name.Profile(ID);
-}
-
void StmtProfiler::VisitTemplateArguments(const TemplateArgumentLoc *Args,
unsigned NumArgs) {
ID.AddInteger(NumArgs);
@@ -1720,6 +1782,11 @@
void Stmt::Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
bool Canonical) const {
- StmtProfiler Profiler(ID, Context, Canonical);
+ StmtProfilerWithPointers Profiler(ID, Context, Canonical);
+ Profiler.Visit(this);
+}
+
+void Stmt::ODRProfile(llvm::FoldingSetNodeID &ID) const {
+ StmtProfilerWithoutPointers Profiler(ID);
Profiler.Visit(this);
}
Index: lib/AST/DeclCXX.cpp
===================================================================
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -16,6 +16,7 @@
#include "clang/AST/ASTMutationListener.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DeclVisitor.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/TypeLoc.h"
@@ -71,8 +72,8 @@
ImplicitCopyAssignmentHasConstParam(true),
HasDeclaredCopyConstructorWithConstParam(false),
HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
- IsParsingBaseSpecifiers(false), NumBases(0), NumVBases(0), Bases(),
- VBases(), Definition(D), FirstFriend() {}
+ IsParsingBaseSpecifiers(false), ODRHash(0), NumBases(0), NumVBases(0),
+ Bases(), VBases(), Definition(D), FirstFriend() {}
CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
return Bases.get(Definition->getASTContext().getExternalSource());
@@ -371,6 +372,253 @@
data().IsParsingBaseSpecifiers = false;
}
+// This DeclVisitor computes a hash for the common Decl types
+// encountered from a CXXRecordDecl.
+class ODRDeclVisitor : public DeclVisitor<ODRDeclVisitor> {
+ typedef DeclVisitor<ODRDeclVisitor> Inherited;
+ llvm::FoldingSetNodeID &ID;
+
+public:
+ ODRDeclVisitor(llvm::FoldingSetNodeID &ID)
+ : ID(ID) {}
+
+ unsigned ComputeHash() { return ID.ComputeHash(); }
+
+ void Visit(Decl *D) {
+ if (!D)
+ return;
+ if (D->isImplicit())
+ return;
+ if (D->isInvalidDecl())
+ return;
+ ID.AddInteger(D->getKind());
+ ID.AddInteger(D->hasAttrs());
+
+ if (auto *DC = dyn_cast<DeclContext>(D))
+ for (auto *SubD : DC->decls())
+ Visit(SubD);
+
+ Inherited::Visit(D);
+ }
+
+ void VisitLabelDecl(LabelDecl *D) {
+ Inherited::VisitLabelDecl(D);
+ }
+
+ void VisitEnumDecl(EnumDecl *D) {
+ ID.AddInteger(D->isFixed());
+ if (D->isFixed())
+ VisitType(D->getIntegerType());
+ ID.AddInteger(D->isScoped());
+ ID.AddInteger(D->isScopedUsingClassTag());
+
+ Inherited::VisitEnumDecl(D);
+ }
+
+ void VisitEnumConstantDecl(EnumConstantDecl *D) {
+ if (auto *E = D->getInitExpr())
+ VisitStmt(E);
+
+ Inherited::VisitEnumConstantDecl(D);
+ }
+
+ void VisitNamedDecl(NamedDecl *D) {
+ if (IdentifierInfo *II = D->getIdentifier()) {
+ ID.AddString(II->getName());
+ }
+ Inherited::VisitNamedDecl(D);
+ }
+
+ void VisitValueDecl(ValueDecl *D) {
+ VisitType(D->getType());
+ Inherited::VisitValueDecl(D);
+ }
+
+ void VisitAccessSpecDecl(AccessSpecDecl *D) {
+ ID.AddInteger(D->getAccess());
+ Inherited::VisitAccessSpecDecl(D);
+ }
+
+ void VisitFriendDecl(FriendDecl *D) {
+ if (TypeSourceInfo *TSI = D->getFriendType())
+ VisitType(TSI->getType());
+ unsigned NumLists = D->getFriendTypeNumTemplateParameterLists();
+ ID.AddInteger(NumLists);
+ for (unsigned i = 0; i < NumLists; ++i)
+ VisitTemplateParameterList(D->getFriendTypeTemplateParameterList(i));
+ Inherited::VisitFriendDecl(D);
+ }
+
+ void VisitStaticAssertDecl(StaticAssertDecl *D) {
+ VisitStmt(D->getAssertExpr());
+ ID.AddString(D->getMessage()->getString());
+ Inherited::VisitStaticAssertDecl(D);
+ }
+
+ void VisitTypedefNameDecl(TypedefNameDecl *D) {
+ VisitType(D->getUnderlyingType());
+
+ Inherited::VisitTypedefNameDecl(D);
+ }
+
+ void VisitFunctionDecl(FunctionDecl *D) {
+ if (D->hasBody())
+ VisitStmt(D->getBody());
+ else
+ ID.AddInteger(false);
+
+ ID.AddInteger(D->getStorageClass());
+ ID.AddInteger(D->isInlineSpecified());
+ ID.AddInteger(D->isVirtualAsWritten());
+ ID.AddInteger(D->isPure());
+ ID.AddInteger(D->isDeletedAsWritten());
+
+ Inherited::VisitFunctionDecl(D);
+ }
+
+ void VisitCXXMethodDecl(CXXMethodDecl *D) {
+ ID.AddInteger(D->isStatic());
+ ID.AddInteger(D->isInstance());
+ ID.AddInteger(D->isConst());
+ ID.AddInteger(D->isVolatile());
+ Inherited::VisitCXXMethodDecl(D);
+ }
+
+ void VisitCXXConstructorDecl(CXXConstructorDecl *D) {
+ ID.AddInteger(D->isExplicitSpecified());
+ ID.AddInteger(D->getNumCtorInitializers());
+ for (auto Initializer : D->inits()) {
+ if (Initializer->isWritten()) {
+ Initializer->getInit()->ODRProfile(ID);
+ }
+ }
+ Inherited::VisitCXXConstructorDecl(D);
+ }
+
+
+ void VisitCXXConversionDecl(CXXConversionDecl *D) {
+ VisitType(D->getConversionType());
+ ID.AddInteger(D->isExplicitSpecified());
+ Inherited::VisitCXXConversionDecl(D);
+ }
+
+ void VisitFieldDecl(FieldDecl *D) {
+ ID.AddInteger(D->isMutable());
+ ID.AddInteger(D->isBitField());
+ if (D->isBitField())
+ D->getBitWidth()->ODRProfile(ID);
+ Inherited::VisitFieldDecl(D);
+ }
+
+ void VisitTemplateDecl(TemplateDecl *D) {
+ if (NamedDecl *ND = D->getTemplatedDecl())
+ Visit(ND);
+ auto *Parameters = D->getTemplateParameters();
+ ID.AddInteger(Parameters->size());
+ for (auto *ND : *Parameters)
+ Visit(ND);
+ Inherited::VisitTemplateDecl(D);
+ }
+
+ void VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
+ Inherited::VisitFunctionTemplateDecl(D);
+ }
+
+ void VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
+ if (D->hasDefaultArgument())
+ VisitType(D->getDefaultArgument());
+ Inherited::VisitTemplateTypeParmDecl(D);
+ }
+
+ void VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
+ if (D->hasDefaultArgument())
+ D->getDefaultArgument()->ODRProfile(ID);
+ Inherited::VisitNonTypeTemplateParmDecl(D);
+ }
+
+ void VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
+ if (D->hasDefaultArgument())
+ VisitTemplateArgument(D->getDefaultArgument().getArgument());
+ Inherited::VisitTemplateTemplateParmDecl(D);
+ }
+
+ void VisitStmt(Stmt *S) {
+ if (!S)
+ return;
+ S->ODRProfile(ID);
+ }
+
+ void VisitTemplateParameterList(TemplateParameterList *TPL) {
+ if (!TPL)
+ return;
+ for (auto * ND : TPL->asArray()) {
+ Visit(ND);
+ }
+ }
+
+ void VisitTemplateArgument(const TemplateArgument& TA) {
+ switch (TA.getKind()) {
+ case TemplateArgument::Null:
+ llvm_unreachable("Require valid TemplateArgument");
+ case TemplateArgument::Type:
+ VisitType(TA.getAsType());
+ return;
+ case TemplateArgument::Declaration:
+ Visit(TA.getAsDecl());
+ return;
+ case TemplateArgument::NullPtr:
+ ID.AddInteger(0);
+ return;
+ case TemplateArgument::Integral:
+ TA.getAsIntegral().Profile(ID);
+ VisitType(TA.getIntegralType());
+ break;
+ case TemplateArgument::Template:
+ case TemplateArgument::TemplateExpansion:
+ Visit(TA.getAsTemplateOrTemplatePattern().getAsTemplateDecl());
+ break;
+ case TemplateArgument::Expression:
+ TA.getAsExpr()->ODRProfile(ID);
+ break;
+ case TemplateArgument::Pack:
+ ID.AddInteger(TA.pack_size());
+ for (auto SubTA : TA.pack_elements())
+ VisitTemplateArgument(SubTA);
+ break;
+ }
+ }
+
+ void VisitType(QualType T) {
+ // TODO: String type is used instead of a proper hash for the type.
+ // Eventually, a hash built from a type visitor should be used.
+ LangOptions options;
+ PrintingPolicy policy(options);
+ policy.AnonymousTagLocations = false;
+ ID.AddString(T.getAsString(policy));
+ }
+};
+
+void CXXRecordDecl::computeODRHash() {
+ if (!DefinitionData)
+ return;
+ llvm::FoldingSetNodeID ID;
+ ODRDeclVisitor Visitor(ID);
+ for (auto *D : decls()) {
+ Visitor.Visit(D);
+ }
+
+ for (auto base : bases()) {
+ ID.AddInteger(base.isVirtual());
+ Visitor.VisitType(base.getType());
+ }
+
+ if (ClassTemplateDecl *TD = getDescribedClassTemplate()) {
+ Visitor.VisitTemplateParameterList(TD->getTemplateParameters());
+ }
+
+ DefinitionData->ODRHash = ID.ComputeHash();
+}
+
void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
// C++11 [class.copy]p11:
// A defaulted copy/move constructor for a class X is defined as
Index: include/clang/AST/Stmt.h
===================================================================
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -422,6 +422,7 @@
/// written in the source.
void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
bool Canonical) const;
+ void ODRProfile(llvm::FoldingSetNodeID &ID) const;
};
/// DeclStmt - Adaptor class for mixing declarations with statements and
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -457,6 +457,9 @@
/// \brief Whether we are currently parsing base specifiers.
unsigned IsParsingBaseSpecifiers : 1;
+ /// \brief A hash of parts of the class to help in ODR checking.
+ unsigned ODRHash;
+
/// \brief The number of base class specifiers in Bases.
unsigned NumBases;
@@ -703,6 +706,9 @@
return data().IsParsingBaseSpecifiers;
}
+ void computeODRHash();
+ unsigned getODRHash() { return data().ODRHash; }
+
/// \brief Sets the base classes of this struct or class.
void setBases(CXXBaseSpecifier const * const *Bases, unsigned NumBases);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
