[clang] e6393ee - Canonicalize declaration pointers when forming APValues.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 4 07:14:45 PDT 2020
To keep the bots greener over the long weekend, I went ahead and reverted
this for now in 7b0332389afd705f46b02fcf87ec3414b8dece34. I'll add a test
for this.
On Fri, Sep 4, 2020 at 10:11 AM Nico Weber <thakis at chromium.org> wrote:
> Hi Richard,
>
> this breaks Wunreachable-code which now ignore "weak" on redeclarations:
>
> thakis at thakis:~/src/llvm-project$ cat foo.cc
> extern "C" void foo(void);
> extern "C" __attribute__((weak)) decltype(foo) foo;
>
> void g(), h();
> void f() {
> if (foo)
> return g();
> h();
> }
> thakis at thakis:~/src/llvm-project$ out/gn/bin/clang -Wunreachable-code -c
> foo.cc
> foo.cc:8:5: warning: code will never be executed [-Wunreachable-code]
> h();
> ^
>
> This seems to be a somewhat common pattern for calling new functions.
>
> (Chromium bug: https://crbug.com/1125102)
>
> On Thu, Sep 3, 2020 at 6:35 PM Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>>
>> Author: Richard Smith
>> Date: 2020-09-03T15:35:12-07:00
>> New Revision: e6393ee813178e9d3306b8e3c6949a4f32f8a2cb
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/e6393ee813178e9d3306b8e3c6949a4f32f8a2cb
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/e6393ee813178e9d3306b8e3c6949a4f32f8a2cb.diff
>>
>> LOG: Canonicalize declaration pointers when forming APValues.
>>
>> References to different declarations of the same entity aren't different
>> values, so shouldn't have different representations.
>>
>> Added:
>>
>>
>> Modified:
>> clang/include/clang/AST/APValue.h
>> clang/lib/AST/APValue.cpp
>> clang/lib/AST/ExprConstant.cpp
>> clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
>> clang/test/OpenMP/ordered_messages.cpp
>>
>> Removed:
>>
>>
>>
>>
>> ################################################################################
>> diff --git a/clang/include/clang/AST/APValue.h
>> b/clang/include/clang/AST/APValue.h
>> index 87e4bd7f84c1..485e6c2602cf 100644
>> --- a/clang/include/clang/AST/APValue.h
>> +++ b/clang/include/clang/AST/APValue.h
>> @@ -174,6 +174,7 @@ class APValue {
>> return !(LHS == RHS);
>> }
>> friend llvm::hash_code hash_value(const LValueBase &Base);
>> + friend struct llvm::DenseMapInfo<LValueBase>;
>>
>> private:
>> PtrTy Ptr;
>> @@ -201,8 +202,7 @@ class APValue {
>>
>> public:
>> LValuePathEntry() : Value() {}
>> - LValuePathEntry(BaseOrMemberType BaseOrMember)
>> - :
>> Value{reinterpret_cast<uintptr_t>(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 2a8834b4db0c..7531229654cf 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<ValueDecl>(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<TypeInfoLValue>())
>> + if (LHS.is<TypeInfoLValue>() || LHS.is<DynamicAllocLValue>())
>> 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<uintptr_t>(BaseOrMember.getOpaqueValue());
>> +}
>> +
>> namespace {
>> struct LVBase {
>> APValue::LValueBase Base;
>> @@ -113,14 +119,16 @@ APValue::LValueBase::operator bool () const {
>>
>> clang::APValue::LValueBase
>> llvm::DenseMapInfo<clang::APValue::LValueBase>::getEmptyKey() {
>> - return clang::APValue::LValueBase(
>> - DenseMapInfo<const ValueDecl*>::getEmptyKey());
>> + clang::APValue::LValueBase B;
>> + B.Ptr = DenseMapInfo<const ValueDecl*>::getEmptyKey();
>> + return B;
>> }
>>
>> clang::APValue::LValueBase
>> llvm::DenseMapInfo<clang::APValue::LValueBase>::getTombstoneKey() {
>> - return clang::APValue::LValueBase(
>> - DenseMapInfo<const ValueDecl*>::getTombstoneKey());
>> + clang::APValue::LValueBase B;
>> + B.Ptr = DenseMapInfo<const ValueDecl*>::getTombstoneKey();
>> + return B;
>> }
>>
>> namespace clang {
>> @@ -757,8 +765,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<ValueDecl>(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/ExprConstant.cpp
>> b/clang/lib/AST/ExprConstant.cpp
>> index e8f132dd4803..8e43b62662ee 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<const ValueDecl*>() == VD) {
>> + if (declaresSameEntity(Info.EvaluatingDecl.dyn_cast<const ValueDecl
>> *>(),
>> + 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 8d51dbde7177..3720b277af7a 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 f6b9dbd6d27f..8a3a86443eb8 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 at -2 {{declared here}}
>> +#endif
>>
>> template <class T>
>> T foo() {
>> @@ -176,7 +179,7 @@ T foo() {
>>
>> int foo() {
>> #if __cplusplus >= 201103L
>> -// expected-note at -2 2 {{declared here}}
>> +// expected-note at -2 {{declared here}}
>> #endif
>> int k;
>> #pragma omp for ordered
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200904/d8c9aa06/attachment-0001.html>
More information about the cfe-commits
mailing list