[clang] c31d6b4 - [ODRHash] Hash type-as-written
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 30 20:08:14 PDT 2023
Author: Chuanqi Xu
Date: 2023-07-31T11:05:47+08:00
New Revision: c31d6b4ef135098280b0ebb93e95b258a0d372ca
URL: https://github.com/llvm/llvm-project/commit/c31d6b4ef135098280b0ebb93e95b258a0d372ca
DIFF: https://github.com/llvm/llvm-project/commit/c31d6b4ef135098280b0ebb93e95b258a0d372ca.diff
LOG: [ODRHash] Hash type-as-written
Close https://github.com/llvm/llvm-project/issues/63947
Close https://github.com/llvm/llvm-project/issues/63595
This is suggested by @rsmith in
https://reviews.llvm.org/D154324#inline-1508868
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D156210
Added:
clang/test/Modules/pr63595.cppm
Modified:
clang/lib/AST/ODRHash.cpp
clang/test/Modules/odr_hash-gnu.cpp
clang/test/Modules/odr_hash.cpp
clang/test/Modules/odr_hash.mm
Removed:
################################################################################
diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index 507fb0b49f8ad3..5eab315793b411 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -295,9 +295,9 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
}
void VisitValueDecl(const ValueDecl *D) {
- if (!isa<FunctionDecl>(D)) {
- AddQualType(D->getType());
- }
+ if (auto *DD = dyn_cast<DeclaratorDecl>(D); DD && DD->getTypeSourceInfo())
+ AddQualType(DD->getTypeSourceInfo()->getType());
+
Inherited::VisitValueDecl(D);
}
@@ -351,7 +351,7 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
void VisitObjCPropertyDecl(const ObjCPropertyDecl *D) {
ID.AddInteger(D->getPropertyAttributes());
ID.AddInteger(D->getPropertyImplementation());
- AddQualType(D->getType());
+ AddQualType(D->getTypeSourceInfo()->getType());
AddDecl(D);
Inherited::VisitObjCPropertyDecl(D);
@@ -394,7 +394,9 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
AddDecl(Method);
- AddQualType(Method->getReturnType());
+ if (Method->getReturnTypeSourceInfo())
+ AddQualType(Method->getReturnTypeSourceInfo()->getType());
+
ID.AddInteger(Method->param_size());
for (auto Param : Method->parameters())
Hash.AddSubDecl(Param);
@@ -593,7 +595,7 @@ void ODRHash::AddCXXRecordDecl(const CXXRecordDecl *Record) {
ID.AddInteger(Record->getNumBases());
auto Bases = Record->bases();
for (const auto &Base : Bases) {
- AddQualType(Base.getType());
+ AddQualType(Base.getTypeSourceInfo()->getType());
ID.AddInteger(Base.isVirtual());
ID.AddInteger(Base.getAccessSpecifierAsWritten());
}
@@ -912,29 +914,7 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
void VisitType(const Type *T) {}
void VisitAdjustedType(const AdjustedType *T) {
- QualType Original = T->getOriginalType();
- QualType Adjusted = T->getAdjustedType();
-
- // The original type and pointee type can be the same, as in the case of
- // function pointers decaying to themselves. Set a bool and only process
- // the type once, to prevent doubling the work.
- SplitQualType split = Adjusted.split();
- if (auto Pointer = dyn_cast<PointerType>(split.Ty)) {
- if (Pointer->getPointeeType() == Original) {
- Hash.AddBoolean(true);
- ID.AddInteger(split.Quals.getAsOpaqueValue());
- AddQualType(Original);
- VisitType(T);
- return;
- }
- }
-
- // The original type and pointee type are
diff erent, such as in the case
- // of a array decaying to an element pointer. Set a bool to false and
- // process both types.
- Hash.AddBoolean(false);
- AddQualType(Original);
- AddQualType(Adjusted);
+ AddQualType(T->getOriginalType());
VisitType(T);
}
@@ -973,7 +953,6 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
void VisitAttributedType(const AttributedType *T) {
ID.AddInteger(T->getAttrKind());
AddQualType(T->getModifiedType());
- AddQualType(T->getEquivalentType());
VisitType(T);
}
@@ -995,7 +974,6 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
void VisitDecltypeType(const DecltypeType *T) {
AddStmt(T->getUnderlyingExpr());
- AddQualType(T->getUnderlyingType());
VisitType(T);
}
@@ -1184,31 +1162,12 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
void VisitTypedefType(const TypedefType *T) {
AddDecl(T->getDecl());
- QualType UnderlyingType = T->getDecl()->getUnderlyingType();
- VisitQualifiers(UnderlyingType.getQualifiers());
- while (true) {
- if (const TypedefType *Underlying =
- dyn_cast<TypedefType>(UnderlyingType.getTypePtr())) {
- UnderlyingType = Underlying->getDecl()->getUnderlyingType();
- continue;
- }
- if (const ElaboratedType *Underlying =
- dyn_cast<ElaboratedType>(UnderlyingType.getTypePtr())) {
- UnderlyingType = Underlying->getNamedType();
- continue;
- }
-
- break;
- }
- AddType(UnderlyingType.getTypePtr());
VisitType(T);
}
void VisitTypeOfExprType(const TypeOfExprType *T) {
AddStmt(T->getUnderlyingExpr());
Hash.AddBoolean(T->isSugared());
- if (T->isSugared())
- AddQualType(T->desugar());
VisitType(T);
}
diff --git a/clang/test/Modules/odr_hash-gnu.cpp b/clang/test/Modules/odr_hash-gnu.cpp
index bb5ad9da383883..f87b2b841f905f 100644
--- a/clang/test/Modules/odr_hash-gnu.cpp
+++ b/clang/test/Modules/odr_hash-gnu.cpp
@@ -65,9 +65,10 @@ Invalid1 i1;
// expected-error at first.h:* {{'Types::TypeOfExpr::Invalid1' has
diff erent definitions in
diff erent modules; first
diff erence is definition in module 'FirstModule' found field 'x' with type 'typeof (1 + 2)' (aka 'int')}}
// expected-note at second.h:* {{but in 'SecondModule' found field 'x' with type 'typeof (3)' (aka 'int')}}
Invalid2 i2;
-// expected-error at second.h:* {{'Types::TypeOfExpr::Invalid2::x' from module 'SecondModule' is not present in definition of 'Types::TypeOfExpr::Invalid2' in module 'FirstModule'}}
-// expected-note at first.h:* {{declaration of 'x' does not match}}
+
Valid v;
+
+// FIXME: We should diagnose the
diff erent definitions of `global`.
#endif
} // namespace TypeOfExpr
@@ -113,8 +114,9 @@ Invalid2 i2;
// expected-error at first.h:* {{'Types::TypeOf::Invalid2' has
diff erent definitions in
diff erent modules; first
diff erence is definition in module 'FirstModule' found field 'x' with type 'typeof(int)' (aka 'int')}}
// expected-note at second.h:* {{but in 'SecondModule' found field 'x' with type 'typeof(I)' (aka 'int')}}
Invalid3 i3;
-// expected-error at second.h:* {{'Types::TypeOf::Invalid3::x' from module 'SecondModule' is not present in definition of 'Types::TypeOf::Invalid3' in module 'FirstModule'}}
-// expected-note at first.h:* {{declaration of 'x' does not match}}
+
+// FIXME: We should reject the `Invalid3` due to the inconsistent definition of `T`.
+
Valid v;
#endif
} // namespace TypeOf
diff --git a/clang/test/Modules/odr_hash.cpp b/clang/test/Modules/odr_hash.cpp
index c5bf6e873e52ec..7954a1f5f2e50b 100644
--- a/clang/test/Modules/odr_hash.cpp
+++ b/clang/test/Modules/odr_hash.cpp
@@ -1046,8 +1046,7 @@ struct S3 {
};
#else
S3 s3;
-// expected-error at first.h:* {{'TypeDef::S3::a' from module 'FirstModule' is not present in definition of 'TypeDef::S3' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'a' does not match}}
+// FIXME: We should reject the merge of `S3` due to the inconsistent definition of `T`.
#endif
#if defined(FIRST)
@@ -1172,8 +1171,7 @@ struct S3 {
};
#else
S3 s3;
-// expected-error at first.h:* {{'Using::S3::a' from module 'FirstModule' is not present in definition of 'Using::S3' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'a' does not match}}
+// FIXME: We should reject the merge of `S3` due to the inconsistent definition of `T`.
#endif
#if defined(FIRST)
@@ -1361,8 +1359,6 @@ struct S1 {
};
#else
S1 s1;
-// expected-error at first.h:* {{'ElaboratedType::S1::x' from module 'FirstModule' is not present in definition of 'ElaboratedType::S1' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'x' does not match}}
#endif
#define DECLS \
@@ -3616,8 +3612,8 @@ auto function1 = invalid1;
// expected-error at second.h:* {{'Types::Decltype::invalid1' has
diff erent definitions in
diff erent modules; definition in module 'SecondModule' first
diff erence is function body}}
// expected-note at first.h:* {{but in 'FirstModule' found a
diff erent body}}
auto function2 = invalid2;
-// expected-error at second.h:* {{'Types::Decltype::invalid2' has
diff erent definitions in
diff erent modules; definition in module 'SecondModule' first
diff erence is function body}}
-// expected-note at first.h:* {{but in 'FirstModule' found a
diff erent body}}
+// FIXME: We should reject the merge of `invalid2` and diagnose about the
+// inconsistent definition of `global`.
auto function3 = valid;
#endif
} // namespace Decltype
@@ -3700,8 +3696,7 @@ void valid() {
}
#else
auto function1 = invalid1;
-// expected-error at second.h:* {{'Types::DeducedTemplateSpecialization::invalid1' has
diff erent definitions in
diff erent modules; definition in module 'SecondModule' first
diff erence is function body}}
-// expected-note at first.h:* {{but in 'FirstModule' found a
diff erent body}}
+// FIXME: We should reject the merge of `invalid1` due to the inconsistent definition.
auto function2 = invalid2;
// expected-error at second.h:* {{'Types::DeducedTemplateSpecialization::invalid2' has
diff erent definitions in
diff erent modules; definition in module 'SecondModule' first
diff erence is function body}}
// expected-note at first.h:* {{but in 'FirstModule' found a
diff erent body}}
@@ -4946,8 +4941,42 @@ class S4 {
#else
S4 s4;
#endif
+
+#if defined(FIRST)
+namespace NS5 {
+struct T5;
+} // namespace NS5
+class S5 {
+ NS5::T5* t = 0;
+};
+#elif defined(SECOND)
+namespace NS5 {
+typedef struct T5_Other {} T5;
+} // namespace NS4
+class S5 {
+ NS5::T5* t = 0;
+};
+#else
+S5 s5;
+// expected-error at first.h:* {{'TypedefStruct::S5::t' from module 'FirstModule' is not present in definition of 'TypedefStruct::S5' in module 'SecondModule'}}
+// expected-note at second.h:* {{declaration of 't' does not match}}
+#endif
} // namespace TypedefStruct
+#if defined (FIRST)
+typedef int T;
+namespace A {
+ struct X { T n; };
+}
+#elif defined(SECOND)
+namespace A {
+ typedef int T;
+ struct X { T n; };
+}
+#else
+A::X x;
+#endif
+
// Keep macros contained to one file.
#ifdef FIRST
#undef FIRST
diff --git a/clang/test/Modules/odr_hash.mm b/clang/test/Modules/odr_hash.mm
index 276bd58624eca4..9c6454a790edb8 100644
--- a/clang/test/Modules/odr_hash.mm
+++ b/clang/test/Modules/odr_hash.mm
@@ -112,8 +112,7 @@ void valid() {
// expected-error at second.h:* {{Types::Attributed::invalid1' has
diff erent definitions in
diff erent modules; definition in module 'SecondModule' first
diff erence is function body}}
// expected-note at first.h:* {{but in 'FirstModule' found a
diff erent body}}
auto function2 = invalid2;
-// expected-error at second.h:* {{'Types::Attributed::invalid2' has
diff erent definitions in
diff erent modules; definition in module 'SecondModule' first
diff erence is function body}}
-// expected-note at first.h:* {{but in 'FirstModule' found a
diff erent body}}
+
auto function3 = valid;
#endif
} // namespace Attributed
@@ -266,12 +265,7 @@ @interface Interface6 <T1 : I1 *, T2 : I2 *> {
}
@end
#else
-// expected-error at first.h:* {{'Interface4::x' from module 'FirstModule' is not present in definition of 'Interface4' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'x' does not match}}
-// expected-error at first.h:* {{'Interface5::y' from module 'FirstModule' is not present in definition of 'Interface5' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'y' does not match}}
-// expected-error at first.h:* {{'Interface6::z' from module 'FirstModule' is not present in definition of 'Interface6' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'z' does not match}}
+
#endif
namespace Types {
@@ -291,14 +285,14 @@ @interface Interface6 <T1 : I1 *, T2 : I2 *> {
};
#else
Invalid1 i1;
-// expected-error at first.h:* {{'Types::ObjCTypeParam::Invalid1::x' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid1' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'x' does not match}}
+
Invalid2 i2;
-// expected-error at first.h:* {{'Types::ObjCTypeParam::Invalid2::y' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid2' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'y' does not match}}
+
Invalid3 i3;
-// expected-error at first.h:* {{'Types::ObjCTypeParam::Invalid3::z' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid3' in module 'SecondModule'}}
-// expected-note at second.h:* {{declaration of 'z' does not match}}
+
+// FIXME: We should reject to merge these structs and diagnose for the
+//
diff erent definitions for Interface4/Interface5/Interface6.
+
#endif
} // namespace ObjCTypeParam
diff --git a/clang/test/Modules/pr63595.cppm b/clang/test/Modules/pr63595.cppm
new file mode 100644
index 00000000000000..13a5f84a3e71f2
--- /dev/null
+++ b/clang/test/Modules/pr63595.cppm
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module1.cppm -o %t/module1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module2.cppm -o %t/module2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/merge.cpp -verify -fsyntax-only
+
+//--- header.h
+namespace NS {
+template <int I>
+class A {
+};
+
+template <template <int I_> class T>
+class B {
+};
+}
+
+//--- module1.h
+namespace NS {
+using C = B<A>;
+}
+struct D : NS::C {
+ using Type = NS::C;
+};
+
+//--- module1.cppm
+// inside NS, using C = B<A>
+module;
+#include "header.h"
+#include "module1.h"
+export module module1;
+export using ::D;
+
+//--- module2.h
+namespace NS {
+using C = B<NS::A>;
+}
+struct D : NS::C {
+ using Type = NS::C;
+};
+
+//--- module2.cppm
+// inside NS, using C = B<NS::A>
+module;
+#include "header.h"
+#include "module2.h"
+export module module2;
+export using ::D;
+
+//--- merge.cpp
+// expected-no-diagnostics
+import module1;
+import module2;
+D d;
More information about the cfe-commits
mailing list