<div dir="ltr">Could there be an issue with Objective C property attributes? <a href="https://crbug.com/1134762">https://crbug.com/1134762</a></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Oct 11, 2020 at 2:11 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</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"><div dir="ltr">Please can you try building with <a href="https://reviews.llvm.org/D89212" target="_blank">https://reviews.llvm.org/D89212</a> applied first, and see if it produces any warnings? If so, you're probably hitting PR47663, and should fix that by moving the 'weak' attribute earlier.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 11 Oct 2020 at 11:18, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</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"><div dir="ltr"><div dir="ltr">On Fri, 9 Oct 2020 at 19:12, Arthur Eubanks <<a href="mailto:aeubanks@google.com" target="_blank">aeubanks@google.com</a>> wrote:<br></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I think this is the cause of <a href="https://crbug.com/1134762" target="_blank">https://crbug.com/1134762</a>. Can we speculatively revert while coming up with a repro? Or would you like a repro first?</div></blockquote><div><br></div><div>If you're blocked on this, sure, please go ahead and revert until you have a repro. But the revert will block ongoing development work, so a reproducer would definitely be appreciated! Per PR47663, there are some pretty fundamental problems with adding the 'weak' attribute "too late", so if that's what's going on, the best approach might be to move the 'weak' attribute to an earlier declaration.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><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" target="_blank">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>
</blockquote></div></div>
</div>
</blockquote></div>
</blockquote></div>