From 905b9ca26c94fa86339451a528cedde5004fc1bb Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 2 Sep 2020 14:42:37 -0700 Subject: [PATCH] Canonicalize declaration pointers when forming APValues. References to different declarations of the same entity aren't different values, so shouldn't have different representations. Recommit of e6393ee813178e9d3306b8e3c6949a4f32f8a2cb with fixed handling for weak declarations. We now look for attributes on the most recent declaration when determining whether a declaration is weak. --- clang/include/clang/AST/APValue.h | 4 ++-- clang/lib/AST/APValue.cpp | 26 +++++++++++++++------- clang/lib/AST/Decl.cpp | 2 +- clang/lib/AST/DeclBase.cpp | 2 +- clang/lib/AST/ExprConstant.cpp | 18 +++++---------- .../test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp | 3 +-- clang/test/OpenMP/ordered_messages.cpp | 5 ++++- 7 files changed, 33 insertions(+), 27 deletions(-) diff --git a/clang/include/clang/AST/APValue.h b/clang/include/clang/AST/APValue.h index 5103cfa..6307f8a 100644 --- a/clang/include/clang/AST/APValue.h +++ b/clang/include/clang/AST/APValue.h @@ -174,6 +174,7 @@ public: return !(LHS == RHS); } friend llvm::hash_code hash_value(const LValueBase &Base); + friend struct llvm::DenseMapInfo; private: PtrTy Ptr; @@ -201,8 +202,7 @@ public: public: LValuePathEntry() : Value() {} - LValuePathEntry(BaseOrMemberType BaseOrMember) - : Value{reinterpret_cast(BaseOrMember.getOpaqueValue())} {} + LValuePathEntry(BaseOrMemberType BaseOrMember); static LValuePathEntry ArrayIndex(uint64_t Index) { LValuePathEntry Result; Result.Value = Index; diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp index 08ae0ff..32d3ff7 100644 --- a/clang/lib/AST/APValue.cpp +++ b/clang/lib/AST/APValue.cpp @@ -38,7 +38,7 @@ static_assert( "Type is insufficiently aligned"); APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned V) - : Ptr(P), Local{I, V} {} + : Ptr(P ? cast(P->getCanonicalDecl()) : nullptr), Local{I, V} {} APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V) : Ptr(P), Local{I, V} {} @@ -82,13 +82,19 @@ bool operator==(const APValue::LValueBase &LHS, const APValue::LValueBase &RHS) { if (LHS.Ptr != RHS.Ptr) return false; - if (LHS.is()) + if (LHS.is() || LHS.is()) return true; return LHS.Local.CallIndex == RHS.Local.CallIndex && LHS.Local.Version == RHS.Local.Version; } } +APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType BaseOrMember) { + if (const Decl *D = BaseOrMember.getPointer()) + BaseOrMember.setPointer(D->getCanonicalDecl()); + Value = reinterpret_cast(BaseOrMember.getOpaqueValue()); +} + namespace { struct LVBase { APValue::LValueBase Base; @@ -113,14 +119,16 @@ APValue::LValueBase::operator bool () const { clang::APValue::LValueBase llvm::DenseMapInfo::getEmptyKey() { - return clang::APValue::LValueBase( - DenseMapInfo::getEmptyKey()); + clang::APValue::LValueBase B; + B.Ptr = DenseMapInfo::getEmptyKey(); + return B; } clang::APValue::LValueBase llvm::DenseMapInfo::getTombstoneKey() { - return clang::APValue::LValueBase( - DenseMapInfo::getTombstoneKey()); + clang::APValue::LValueBase B; + B.Ptr = DenseMapInfo::getTombstoneKey(); + return B; } namespace clang { @@ -773,8 +781,10 @@ void APValue::MakeMemberPointer(const ValueDecl *Member, bool IsDerivedMember, assert(isAbsent() && "Bad state change"); MemberPointerData *MPD = new ((void*)(char*)Data.buffer) MemberPointerData; Kind = MemberPointer; - MPD->MemberAndIsDerivedMember.setPointer(Member); + MPD->MemberAndIsDerivedMember.setPointer( + Member ? cast(Member->getCanonicalDecl()) : nullptr); MPD->MemberAndIsDerivedMember.setInt(IsDerivedMember); MPD->resizePath(Path.size()); - memcpy(MPD->getPath(), Path.data(), Path.size()*sizeof(const CXXRecordDecl*)); + for (unsigned I = 0; I != Path.size(); ++I) + MPD->getPath()[I] = Path[I]->getCanonicalDecl(); } diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 9815f06..b446bf0 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -4706,7 +4706,7 @@ char *Buffer = new (getASTContext(), 1) char[Name.size() + 1]; void ValueDecl::anchor() {} bool ValueDecl::isWeak() const { - for (const auto *I : attrs()) + for (const auto *I : getMostRecentDecl()->attrs()) if (isa(I) || isa(I)) return true; diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index f4314d0..ab2b55c 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -720,7 +720,7 @@ bool Decl::isWeakImported() const { if (!canBeWeakImported(IsDefinition)) return false; - for (const auto *A : attrs()) { + for (const auto *A : getMostRecentDecl()->attrs()) { if (isa(A)) return true; diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index e8f132d..8e43b62 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -1978,18 +1978,11 @@ static bool HasSameBase(const LValue &A, const LValue &B) { return false; if (A.getLValueBase().getOpaqueValue() != - B.getLValueBase().getOpaqueValue()) { - const Decl *ADecl = GetLValueBaseDecl(A); - if (!ADecl) - return false; - const Decl *BDecl = GetLValueBaseDecl(B); - if (!BDecl || ADecl->getCanonicalDecl() != BDecl->getCanonicalDecl()) - return false; - } + B.getLValueBase().getOpaqueValue()) + return false; - return IsGlobalLValue(A.getLValueBase()) || - (A.getLValueCallIndex() == B.getLValueCallIndex() && - A.getLValueVersion() == B.getLValueVersion()); + return A.getLValueCallIndex() == B.getLValueCallIndex() && + A.getLValueVersion() == B.getLValueVersion(); } static void NoteLValueLocation(EvalInfo &Info, APValue::LValueBase Base) { @@ -3108,7 +3101,8 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, // If we're currently evaluating the initializer of this declaration, use that // in-flight value. - if (Info.EvaluatingDecl.dyn_cast() == VD) { + if (declaresSameEntity(Info.EvaluatingDecl.dyn_cast(), + VD)) { Result = Info.EvaluatingDeclValue; return true; } diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp index 8d51dbd..3720b27 100644 --- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp @@ -24,11 +24,10 @@ constexpr double &ni3; // expected-error {{declaration of reference variable 'ni constexpr int nc1 = i; // expected-error {{constexpr variable 'nc1' must be initialized by a constant expression}} expected-note {{read of non-const variable 'i' is not allowed in a constant expression}} constexpr C nc2 = C(); // expected-error {{cannot have non-literal type 'const C'}} -int &f(); // expected-note {{declared here}} +int &f(); // expected-note 2{{declared here}} constexpr int &nc3 = f(); // expected-error {{constexpr variable 'nc3' must be initialized by a constant expression}} expected-note {{non-constexpr function 'f' cannot be used in a constant expression}} constexpr int nc4(i); // expected-error {{constexpr variable 'nc4' must be initialized by a constant expression}} expected-note {{read of non-const variable 'i' is not allowed in a constant expression}} constexpr C nc5((C())); // expected-error {{cannot have non-literal type 'const C'}} -int &f(); // expected-note {{here}} constexpr int &nc6(f()); // expected-error {{constexpr variable 'nc6' must be initialized by a constant expression}} expected-note {{non-constexpr function 'f'}} struct pixel { diff --git a/clang/test/OpenMP/ordered_messages.cpp b/clang/test/OpenMP/ordered_messages.cpp index f6b9dbd..8a3a864 100644 --- a/clang/test/OpenMP/ordered_messages.cpp +++ b/clang/test/OpenMP/ordered_messages.cpp @@ -16,6 +16,9 @@ void xxx(int argc) { } int foo(); +#if __cplusplus >= 201103L +// expected-note@-2 {{declared here}} +#endif template T foo() { @@ -176,7 +179,7 @@ T foo() { int foo() { #if __cplusplus >= 201103L -// expected-note@-2 2 {{declared here}} +// expected-note@-2 {{declared here}} #endif int k; #pragma omp for ordered -- 2.7.4