[clang] e6393ee - Canonicalize declaration pointers when forming APValues.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 4 12:37:18 PDT 2020
Thanks for the revert and the test. I'd imagine that we're forgetting that
we need to ask the most recent declaration whether it's weak -- isWeak()
can (presumably) change along the redecl chain. Fun :)
On Fri, 4 Sep 2020, 07:36 Nico Weber, <thakis at chromium.org> wrote:
> Actually in 2a03f270d69cf1079feb029f84727288e217588a
>
> On Fri, Sep 4, 2020 at 10:27 AM Nico Weber <thakis at chromium.org> wrote:
>
>> Test added in 85a9f6512a3cff797f1964c36c59d53e97a680c4
>>
>> On Fri, Sep 4, 2020 at 10:14 AM Nico Weber <thakis at chromium.org> wrote:
>>
>>> 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/45333486/attachment.html>
More information about the cfe-commits
mailing list