[clang] e6393ee - Canonicalize declaration pointers when forming APValues.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 4 07:27:35 PDT 2020
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/5b98de7f/attachment.html>
More information about the cfe-commits
mailing list