<div dir="ltr">I think this is the cause of <a href="https://crbug.com/1134762">https://crbug.com/1134762</a>. Can we speculatively revert while coming up with a repro? Or would you like a repro first?</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Sep 27, 2020 at 7:06 PM Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Richard Smith<br>
Date: 2020-09-27T19:05:26-07:00<br>
New Revision: 9dcd96f728863d40d6f5922ed52732fdd728fb5f<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f.diff</a><br>
<br>
LOG: Canonicalize declaration pointers when forming APValues.<br>
<br>
References to different declarations of the same entity aren't different<br>
values, so shouldn't have different representations.<br>
<br>
Recommit of e6393ee813178e9d3306b8e3c6949a4f32f8a2cb with fixed handling<br>
for weak declarations. We now look for attributes on the most recent<br>
declaration when determining whether a declaration is weak. (Second<br>
recommit with further fixes for mishandling of weak declarations. Our<br>
behavior here is fundamentally unsound -- see PR47663 -- but this<br>
approach attempts to not make things worse.)<br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
clang/include/clang/AST/APValue.h<br>
clang/lib/AST/APValue.cpp<br>
clang/lib/AST/Decl.cpp<br>
clang/lib/AST/DeclBase.cpp<br>
clang/lib/AST/ExprConstant.cpp<br>
clang/lib/CodeGen/CGExprConstant.cpp<br>
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp<br>
clang/test/CodeGenCXX/weak-external.cpp<br>
clang/test/OpenMP/ordered_messages.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang/include/clang/AST/APValue.h b/clang/include/clang/AST/APValue.h<br>
index 5103cfa8604e..6307f8a92e5a 100644<br>
--- a/clang/include/clang/AST/APValue.h<br>
+++ b/clang/include/clang/AST/APValue.h<br>
@@ -174,6 +174,7 @@ class APValue {<br>
return !(LHS == RHS);<br>
}<br>
friend llvm::hash_code hash_value(const LValueBase &Base);<br>
+ friend struct llvm::DenseMapInfo<LValueBase>;<br>
<br>
private:<br>
PtrTy Ptr;<br>
@@ -201,8 +202,7 @@ class APValue {<br>
<br>
public:<br>
LValuePathEntry() : Value() {}<br>
- LValuePathEntry(BaseOrMemberType BaseOrMember)<br>
- : Value{reinterpret_cast<uintptr_t>(BaseOrMember.getOpaqueValue())} {}<br>
+ LValuePathEntry(BaseOrMemberType BaseOrMember);<br>
static LValuePathEntry ArrayIndex(uint64_t Index) {<br>
LValuePathEntry Result;<br>
Result.Value = Index;<br>
<br>
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp<br>
index 08ae0ff3c67d..32d3ff7ce1d0 100644<br>
--- a/clang/lib/AST/APValue.cpp<br>
+++ b/clang/lib/AST/APValue.cpp<br>
@@ -38,7 +38,7 @@ static_assert(<br>
"Type is insufficiently aligned");<br>
<br>
APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned V)<br>
- : Ptr(P), Local{I, V} {}<br>
+ : Ptr(P ? cast<ValueDecl>(P->getCanonicalDecl()) : nullptr), Local{I, V} {}<br>
APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V)<br>
: Ptr(P), Local{I, V} {}<br>
<br>
@@ -82,13 +82,19 @@ bool operator==(const APValue::LValueBase &LHS,<br>
const APValue::LValueBase &RHS) {<br>
if (LHS.Ptr != RHS.Ptr)<br>
return false;<br>
- if (LHS.is<TypeInfoLValue>())<br>
+ if (LHS.is<TypeInfoLValue>() || LHS.is<DynamicAllocLValue>())<br>
return true;<br>
return LHS.Local.CallIndex == RHS.Local.CallIndex &&<br>
LHS.Local.Version == RHS.Local.Version;<br>
}<br>
}<br>
<br>
+APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType BaseOrMember) {<br>
+ if (const Decl *D = BaseOrMember.getPointer())<br>
+ BaseOrMember.setPointer(D->getCanonicalDecl());<br>
+ Value = reinterpret_cast<uintptr_t>(BaseOrMember.getOpaqueValue());<br>
+}<br>
+<br>
namespace {<br>
struct LVBase {<br>
APValue::LValueBase Base;<br>
@@ -113,14 +119,16 @@ APValue::LValueBase::operator bool () const {<br>
<br>
clang::APValue::LValueBase<br>
llvm::DenseMapInfo<clang::APValue::LValueBase>::getEmptyKey() {<br>
- return clang::APValue::LValueBase(<br>
- DenseMapInfo<const ValueDecl*>::getEmptyKey());<br>
+ clang::APValue::LValueBase B;<br>
+ B.Ptr = DenseMapInfo<const ValueDecl*>::getEmptyKey();<br>
+ return B;<br>
}<br>
<br>
clang::APValue::LValueBase<br>
llvm::DenseMapInfo<clang::APValue::LValueBase>::getTombstoneKey() {<br>
- return clang::APValue::LValueBase(<br>
- DenseMapInfo<const ValueDecl*>::getTombstoneKey());<br>
+ clang::APValue::LValueBase B;<br>
+ B.Ptr = DenseMapInfo<const ValueDecl*>::getTombstoneKey();<br>
+ return B;<br>
}<br>
<br>
namespace clang {<br>
@@ -773,8 +781,10 @@ void APValue::MakeMemberPointer(const ValueDecl *Member, bool IsDerivedMember,<br>
assert(isAbsent() && "Bad state change");<br>
MemberPointerData *MPD = new ((void*)(char*)Data.buffer) MemberPointerData;<br>
Kind = MemberPointer;<br>
- MPD->MemberAndIsDerivedMember.setPointer(Member);<br>
+ MPD->MemberAndIsDerivedMember.setPointer(<br>
+ Member ? cast<ValueDecl>(Member->getCanonicalDecl()) : nullptr);<br>
MPD->MemberAndIsDerivedMember.setInt(IsDerivedMember);<br>
MPD->resizePath(Path.size());<br>
- memcpy(MPD->getPath(), Path.data(), Path.size()*sizeof(const CXXRecordDecl*));<br>
+ for (unsigned I = 0; I != Path.size(); ++I)<br>
+ MPD->getPath()[I] = Path[I]->getCanonicalDecl();<br>
}<br>
<br>
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp<br>
index 0ee1399d42df..c96450b8a377 100644<br>
--- a/clang/lib/AST/Decl.cpp<br>
+++ b/clang/lib/AST/Decl.cpp<br>
@@ -4686,11 +4686,9 @@ char *Buffer = new (getASTContext(), 1) char[Name.size() + 1];<br>
void ValueDecl::anchor() {}<br>
<br>
bool ValueDecl::isWeak() const {<br>
- for (const auto *I : attrs())<br>
- if (isa<WeakAttr>(I) || isa<WeakRefAttr>(I))<br>
- return true;<br>
-<br>
- return isWeakImported();<br>
+ auto *MostRecent = getMostRecentDecl();<br>
+ return MostRecent->hasAttr<WeakAttr>() ||<br>
+ MostRecent->hasAttr<WeakRefAttr>() || isWeakImported();<br>
}<br>
<br>
void ImplicitParamDecl::anchor() {}<br>
<br>
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp<br>
index f4314d0bd961..ab2b55c0762e 100644<br>
--- a/clang/lib/AST/DeclBase.cpp<br>
+++ b/clang/lib/AST/DeclBase.cpp<br>
@@ -720,7 +720,7 @@ bool Decl::isWeakImported() const {<br>
if (!canBeWeakImported(IsDefinition))<br>
return false;<br>
<br>
- for (const auto *A : attrs()) {<br>
+ for (const auto *A : getMostRecentDecl()->attrs()) {<br>
if (isa<WeakImportAttr>(A))<br>
return true;<br>
<br>
<br>
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp<br>
index 9bc4afd0b9f8..3bc649b96990 100644<br>
--- a/clang/lib/AST/ExprConstant.cpp<br>
+++ b/clang/lib/AST/ExprConstant.cpp<br>
@@ -1978,18 +1978,11 @@ static bool HasSameBase(const LValue &A, const LValue &B) {<br>
return false;<br>
<br>
if (A.getLValueBase().getOpaqueValue() !=<br>
- B.getLValueBase().getOpaqueValue()) {<br>
- const Decl *ADecl = GetLValueBaseDecl(A);<br>
- if (!ADecl)<br>
- return false;<br>
- const Decl *BDecl = GetLValueBaseDecl(B);<br>
- if (!BDecl || ADecl->getCanonicalDecl() != BDecl->getCanonicalDecl())<br>
- return false;<br>
- }<br>
+ B.getLValueBase().getOpaqueValue())<br>
+ return false;<br>
<br>
- return IsGlobalLValue(A.getLValueBase()) ||<br>
- (A.getLValueCallIndex() == B.getLValueCallIndex() &&<br>
- A.getLValueVersion() == B.getLValueVersion());<br>
+ return A.getLValueCallIndex() == B.getLValueCallIndex() &&<br>
+ A.getLValueVersion() == B.getLValueVersion();<br>
}<br>
<br>
static void NoteLValueLocation(EvalInfo &Info, APValue::LValueBase Base) {<br>
@@ -3158,7 +3151,8 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,<br>
<br>
// If we're currently evaluating the initializer of this declaration, use that<br>
// in-flight value.<br>
- if (Info.EvaluatingDecl.dyn_cast<const ValueDecl*>() == VD) {<br>
+ if (declaresSameEntity(Info.EvaluatingDecl.dyn_cast<const ValueDecl *>(),<br>
+ VD)) {<br>
Result = Info.EvaluatingDeclValue;<br>
return true;<br>
}<br>
<br>
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp<br>
index b0a37531dfe1..bff4a0c38af9 100644<br>
--- a/clang/lib/CodeGen/CGExprConstant.cpp<br>
+++ b/clang/lib/CodeGen/CGExprConstant.cpp<br>
@@ -1877,6 +1877,10 @@ ConstantLValue<br>
ConstantLValueEmitter::tryEmitBase(const APValue::LValueBase &base) {<br>
// Handle values.<br>
if (const ValueDecl *D = base.dyn_cast<const ValueDecl*>()) {<br>
+ // The constant always points to the canonical declaration. We want to look<br>
+ // at properties of the most recent declaration at the point of emission.<br>
+ D = cast<ValueDecl>(D->getMostRecentDecl());<br>
+<br>
if (D->hasAttr<WeakRefAttr>())<br>
return CGM.GetWeakRefReference(D).getPointer();<br>
<br>
<br>
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<br>
index 8d51dbde7177..3720b277af7a 100644<br>
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp<br>
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp<br>
@@ -24,11 +24,10 @@ constexpr double &ni3; // expected-error {{declaration of reference variable 'ni<br>
<br>
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}}<br>
constexpr C nc2 = C(); // expected-error {{cannot have non-literal type 'const C'}}<br>
-int &f(); // expected-note {{declared here}}<br>
+int &f(); // expected-note 2{{declared here}}<br>
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}}<br>
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}}<br>
constexpr C nc5((C())); // expected-error {{cannot have non-literal type 'const C'}}<br>
-int &f(); // expected-note {{here}}<br>
constexpr int &nc6(f()); // expected-error {{constexpr variable 'nc6' must be initialized by a constant expression}} expected-note {{non-constexpr function 'f'}}<br>
<br>
struct pixel {<br>
<br>
diff --git a/clang/test/CodeGenCXX/weak-external.cpp b/clang/test/CodeGenCXX/weak-external.cpp<br>
index a2c53a59dcd5..433fb3c80624 100644<br>
--- a/clang/test/CodeGenCXX/weak-external.cpp<br>
+++ b/clang/test/CodeGenCXX/weak-external.cpp<br>
@@ -64,3 +64,15 @@ class _LIBCPP_EXCEPTION_ABI runtime_error<br>
void dummysymbol() {<br>
throw(std::runtime_error("string"));<br>
}<br>
+<br>
+namespace not_weak_on_first {<br>
+ int func();<br>
+ // CHECK: {{.*}} extern_weak {{.*}} @_ZN17not_weak_on_first4funcEv(<br>
+ int func() __attribute__ ((weak));<br>
+<br>
+ typedef int (*FuncT)();<br>
+<br>
+ extern const FuncT table[] = {<br>
+ func,<br>
+ };<br>
+}<br>
<br>
diff --git a/clang/test/OpenMP/ordered_messages.cpp b/clang/test/OpenMP/ordered_messages.cpp<br>
index f6b9dbd6d27f..8a3a86443eb8 100644<br>
--- a/clang/test/OpenMP/ordered_messages.cpp<br>
+++ b/clang/test/OpenMP/ordered_messages.cpp<br>
@@ -16,6 +16,9 @@ void xxx(int argc) {<br>
}<br>
<br>
int foo();<br>
+#if __cplusplus >= 201103L<br>
+// expected-note@-2 {{declared here}}<br>
+#endif<br>
<br>
template <class T><br>
T foo() {<br>
@@ -176,7 +179,7 @@ T foo() {<br>
<br>
int foo() {<br>
#if __cplusplus >= 201103L<br>
-// expected-note@-2 2 {{declared here}}<br>
+// expected-note@-2 {{declared here}}<br>
#endif<br>
int k;<br>
#pragma omp for ordered<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>