Skip to content

Commit c31d6b4

Browse files
committed
[ODRHash] Hash type-as-written
Close #63947 Close #63595 This is suggested by @rsmith in https://reviews.llvm.org/D154324#inline-1508868 Reviewed By: rsmith Differential Revision: https://reviews.llvm.org/D156210
1 parent fac7c32 commit c31d6b4

File tree

5 files changed

+118
-78
lines changed

5 files changed

+118
-78
lines changed

clang/lib/AST/ODRHash.cpp

Lines changed: 9 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,9 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
295295
}
296296

297297
void VisitValueDecl(const ValueDecl *D) {
298-
if (!isa<FunctionDecl>(D)) {
299-
AddQualType(D->getType());
300-
}
298+
if (auto *DD = dyn_cast<DeclaratorDecl>(D); DD && DD->getTypeSourceInfo())
299+
AddQualType(DD->getTypeSourceInfo()->getType());
300+
301301
Inherited::VisitValueDecl(D);
302302
}
303303

@@ -351,7 +351,7 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
351351
void VisitObjCPropertyDecl(const ObjCPropertyDecl *D) {
352352
ID.AddInteger(D->getPropertyAttributes());
353353
ID.AddInteger(D->getPropertyImplementation());
354-
AddQualType(D->getType());
354+
AddQualType(D->getTypeSourceInfo()->getType());
355355
AddDecl(D);
356356

357357
Inherited::VisitObjCPropertyDecl(D);
@@ -394,7 +394,9 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
394394

395395
AddDecl(Method);
396396

397-
AddQualType(Method->getReturnType());
397+
if (Method->getReturnTypeSourceInfo())
398+
AddQualType(Method->getReturnTypeSourceInfo()->getType());
399+
398400
ID.AddInteger(Method->param_size());
399401
for (auto Param : Method->parameters())
400402
Hash.AddSubDecl(Param);
@@ -593,7 +595,7 @@ void ODRHash::AddCXXRecordDecl(const CXXRecordDecl *Record) {
593595
ID.AddInteger(Record->getNumBases());
594596
auto Bases = Record->bases();
595597
for (const auto &Base : Bases) {
596-
AddQualType(Base.getType());
598+
AddQualType(Base.getTypeSourceInfo()->getType());
597599
ID.AddInteger(Base.isVirtual());
598600
ID.AddInteger(Base.getAccessSpecifierAsWritten());
599601
}
@@ -912,29 +914,7 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
912914
void VisitType(const Type *T) {}
913915

914916
void VisitAdjustedType(const AdjustedType *T) {
915-
QualType Original = T->getOriginalType();
916-
QualType Adjusted = T->getAdjustedType();
917-
918-
// The original type and pointee type can be the same, as in the case of
919-
// function pointers decaying to themselves. Set a bool and only process
920-
// the type once, to prevent doubling the work.
921-
SplitQualType split = Adjusted.split();
922-
if (auto Pointer = dyn_cast<PointerType>(split.Ty)) {
923-
if (Pointer->getPointeeType() == Original) {
924-
Hash.AddBoolean(true);
925-
ID.AddInteger(split.Quals.getAsOpaqueValue());
926-
AddQualType(Original);
927-
VisitType(T);
928-
return;
929-
}
930-
}
931-
932-
// The original type and pointee type are different, such as in the case
933-
// of a array decaying to an element pointer. Set a bool to false and
934-
// process both types.
935-
Hash.AddBoolean(false);
936-
AddQualType(Original);
937-
AddQualType(Adjusted);
917+
AddQualType(T->getOriginalType());
938918

939919
VisitType(T);
940920
}
@@ -973,7 +953,6 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
973953
void VisitAttributedType(const AttributedType *T) {
974954
ID.AddInteger(T->getAttrKind());
975955
AddQualType(T->getModifiedType());
976-
AddQualType(T->getEquivalentType());
977956

978957
VisitType(T);
979958
}
@@ -995,7 +974,6 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
995974

996975
void VisitDecltypeType(const DecltypeType *T) {
997976
AddStmt(T->getUnderlyingExpr());
998-
AddQualType(T->getUnderlyingType());
999977
VisitType(T);
1000978
}
1001979

@@ -1184,31 +1162,12 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
11841162

11851163
void VisitTypedefType(const TypedefType *T) {
11861164
AddDecl(T->getDecl());
1187-
QualType UnderlyingType = T->getDecl()->getUnderlyingType();
1188-
VisitQualifiers(UnderlyingType.getQualifiers());
1189-
while (true) {
1190-
if (const TypedefType *Underlying =
1191-
dyn_cast<TypedefType>(UnderlyingType.getTypePtr())) {
1192-
UnderlyingType = Underlying->getDecl()->getUnderlyingType();
1193-
continue;
1194-
}
1195-
if (const ElaboratedType *Underlying =
1196-
dyn_cast<ElaboratedType>(UnderlyingType.getTypePtr())) {
1197-
UnderlyingType = Underlying->getNamedType();
1198-
continue;
1199-
}
1200-
1201-
break;
1202-
}
1203-
AddType(UnderlyingType.getTypePtr());
12041165
VisitType(T);
12051166
}
12061167

12071168
void VisitTypeOfExprType(const TypeOfExprType *T) {
12081169
AddStmt(T->getUnderlyingExpr());
12091170
Hash.AddBoolean(T->isSugared());
1210-
if (T->isSugared())
1211-
AddQualType(T->desugar());
12121171

12131172
VisitType(T);
12141173
}

clang/test/Modules/odr_hash-gnu.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,10 @@ Invalid1 i1;
6565
// [email protected]:* {{'Types::TypeOfExpr::Invalid1' has different definitions in different modules; first difference is definition in module 'FirstModule' found field 'x' with type 'typeof (1 + 2)' (aka 'int')}}
6666
// [email protected]:* {{but in 'SecondModule' found field 'x' with type 'typeof (3)' (aka 'int')}}
6767
Invalid2 i2;
68-
// [email protected]:* {{'Types::TypeOfExpr::Invalid2::x' from module 'SecondModule' is not present in definition of 'Types::TypeOfExpr::Invalid2' in module 'FirstModule'}}
69-
// [email protected]:* {{declaration of 'x' does not match}}
68+
7069
Valid v;
70+
71+
// FIXME: We should diagnose the different definitions of `global`.
7172
#endif
7273
} // namespace TypeOfExpr
7374

@@ -113,8 +114,9 @@ Invalid2 i2;
113114
// [email protected]:* {{'Types::TypeOf::Invalid2' has different definitions in different modules; first difference is definition in module 'FirstModule' found field 'x' with type 'typeof(int)' (aka 'int')}}
114115
// [email protected]:* {{but in 'SecondModule' found field 'x' with type 'typeof(I)' (aka 'int')}}
115116
Invalid3 i3;
116-
// [email protected]:* {{'Types::TypeOf::Invalid3::x' from module 'SecondModule' is not present in definition of 'Types::TypeOf::Invalid3' in module 'FirstModule'}}
117-
// [email protected]:* {{declaration of 'x' does not match}}
117+
118+
// FIXME: We should reject the `Invalid3` due to the inconsistent definition of `T`.
119+
118120
Valid v;
119121
#endif
120122
} // namespace TypeOf

clang/test/Modules/odr_hash.cpp

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,8 +1046,7 @@ struct S3 {
10461046
};
10471047
#else
10481048
S3 s3;
1049-
// [email protected]:* {{'TypeDef::S3::a' from module 'FirstModule' is not present in definition of 'TypeDef::S3' in module 'SecondModule'}}
1050-
// [email protected]:* {{declaration of 'a' does not match}}
1049+
// FIXME: We should reject the merge of `S3` due to the inconsistent definition of `T`.
10511050
#endif
10521051

10531052
#if defined(FIRST)
@@ -1172,8 +1171,7 @@ struct S3 {
11721171
};
11731172
#else
11741173
S3 s3;
1175-
// [email protected]:* {{'Using::S3::a' from module 'FirstModule' is not present in definition of 'Using::S3' in module 'SecondModule'}}
1176-
// [email protected]:* {{declaration of 'a' does not match}}
1174+
// FIXME: We should reject the merge of `S3` due to the inconsistent definition of `T`.
11771175
#endif
11781176

11791177
#if defined(FIRST)
@@ -1361,8 +1359,6 @@ struct S1 {
13611359
};
13621360
#else
13631361
S1 s1;
1364-
// [email protected]:* {{'ElaboratedType::S1::x' from module 'FirstModule' is not present in definition of 'ElaboratedType::S1' in module 'SecondModule'}}
1365-
// [email protected]:* {{declaration of 'x' does not match}}
13661362
#endif
13671363

13681364
#define DECLS \
@@ -3616,8 +3612,8 @@ auto function1 = invalid1;
36163612
// [email protected]:* {{'Types::Decltype::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
36173613
// [email protected]:* {{but in 'FirstModule' found a different body}}
36183614
auto function2 = invalid2;
3619-
// [email protected]:* {{'Types::Decltype::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
3620-
// [email protected]:* {{but in 'FirstModule' found a different body}}
3615+
// FIXME: We should reject the merge of `invalid2` and diagnose about the
3616+
// inconsistent definition of `global`.
36213617
auto function3 = valid;
36223618
#endif
36233619
} // namespace Decltype
@@ -3700,8 +3696,7 @@ void valid() {
37003696
}
37013697
#else
37023698
auto function1 = invalid1;
3703-
// [email protected]:* {{'Types::DeducedTemplateSpecialization::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
3704-
// [email protected]:* {{but in 'FirstModule' found a different body}}
3699+
// FIXME: We should reject the merge of `invalid1` due to the inconsistent definition.
37053700
auto function2 = invalid2;
37063701
// [email protected]:* {{'Types::DeducedTemplateSpecialization::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
37073702
// [email protected]:* {{but in 'FirstModule' found a different body}}
@@ -4946,8 +4941,42 @@ class S4 {
49464941
#else
49474942
S4 s4;
49484943
#endif
4944+
4945+
#if defined(FIRST)
4946+
namespace NS5 {
4947+
struct T5;
4948+
} // namespace NS5
4949+
class S5 {
4950+
NS5::T5* t = 0;
4951+
};
4952+
#elif defined(SECOND)
4953+
namespace NS5 {
4954+
typedef struct T5_Other {} T5;
4955+
} // namespace NS4
4956+
class S5 {
4957+
NS5::T5* t = 0;
4958+
};
4959+
#else
4960+
S5 s5;
4961+
// [email protected]:* {{'TypedefStruct::S5::t' from module 'FirstModule' is not present in definition of 'TypedefStruct::S5' in module 'SecondModule'}}
4962+
// [email protected]:* {{declaration of 't' does not match}}
4963+
#endif
49494964
} // namespace TypedefStruct
49504965

4966+
#if defined (FIRST)
4967+
typedef int T;
4968+
namespace A {
4969+
struct X { T n; };
4970+
}
4971+
#elif defined(SECOND)
4972+
namespace A {
4973+
typedef int T;
4974+
struct X { T n; };
4975+
}
4976+
#else
4977+
A::X x;
4978+
#endif
4979+
49514980
// Keep macros contained to one file.
49524981
#ifdef FIRST
49534982
#undef FIRST

clang/test/Modules/odr_hash.mm

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ void valid() {
112112
// [email protected]:* {{Types::Attributed::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
113113
// [email protected]:* {{but in 'FirstModule' found a different body}}
114114
auto function2 = invalid2;
115-
// [email protected]:* {{'Types::Attributed::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
116-
// [email protected]:* {{but in 'FirstModule' found a different body}}
115+
117116
auto function3 = valid;
118117
#endif
119118
} // namespace Attributed
@@ -266,12 +265,7 @@ @interface Interface6 <T1 : I1 *, T2 : I2 *> {
266265
}
267266
@end
268267
#else
269-
// [email protected]:* {{'Interface4::x' from module 'FirstModule' is not present in definition of 'Interface4' in module 'SecondModule'}}
270-
// [email protected]:* {{declaration of 'x' does not match}}
271-
// [email protected]:* {{'Interface5::y' from module 'FirstModule' is not present in definition of 'Interface5' in module 'SecondModule'}}
272-
// [email protected]:* {{declaration of 'y' does not match}}
273-
// [email protected]:* {{'Interface6::z' from module 'FirstModule' is not present in definition of 'Interface6' in module 'SecondModule'}}
274-
// [email protected]:* {{declaration of 'z' does not match}}
268+
275269
#endif
276270

277271
namespace Types {
@@ -291,14 +285,14 @@ @interface Interface6 <T1 : I1 *, T2 : I2 *> {
291285
};
292286
#else
293287
Invalid1 i1;
294-
// [email protected]:* {{'Types::ObjCTypeParam::Invalid1::x' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid1' in module 'SecondModule'}}
295-
// [email protected]:* {{declaration of 'x' does not match}}
288+
296289
Invalid2 i2;
297-
// [email protected]:* {{'Types::ObjCTypeParam::Invalid2::y' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid2' in module 'SecondModule'}}
298-
// [email protected]:* {{declaration of 'y' does not match}}
290+
299291
Invalid3 i3;
300-
// [email protected]:* {{'Types::ObjCTypeParam::Invalid3::z' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid3' in module 'SecondModule'}}
301-
// [email protected]:* {{declaration of 'z' does not match}}
292+
293+
// FIXME: We should reject to merge these structs and diagnose for the
294+
// different definitions for Interface4/Interface5/Interface6.
295+
302296
#endif
303297

304298
} // namespace ObjCTypeParam

clang/test/Modules/pr63595.cppm

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// RUN: rm -rf %t
2+
// RUN: mkdir %t
3+
// RUN: split-file %s %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module1.cppm -o %t/module1.pcm
6+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module2.cppm -o %t/module2.pcm
7+
// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/merge.cpp -verify -fsyntax-only
8+
9+
//--- header.h
10+
namespace NS {
11+
template <int I>
12+
class A {
13+
};
14+
15+
template <template <int I_> class T>
16+
class B {
17+
};
18+
}
19+
20+
//--- module1.h
21+
namespace NS {
22+
using C = B<A>;
23+
}
24+
struct D : NS::C {
25+
using Type = NS::C;
26+
};
27+
28+
//--- module1.cppm
29+
// inside NS, using C = B<A>
30+
module;
31+
#include "header.h"
32+
#include "module1.h"
33+
export module module1;
34+
export using ::D;
35+
36+
//--- module2.h
37+
namespace NS {
38+
using C = B<NS::A>;
39+
}
40+
struct D : NS::C {
41+
using Type = NS::C;
42+
};
43+
44+
//--- module2.cppm
45+
// inside NS, using C = B<NS::A>
46+
module;
47+
#include "header.h"
48+
#include "module2.h"
49+
export module module2;
50+
export using ::D;
51+
52+
//--- merge.cpp
53+
// expected-no-diagnostics
54+
import module1;
55+
import module2;
56+
D d;

0 commit comments

Comments
 (0)