r295252 - [Modules] Consider enable_if attrs in isSameEntity.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 23 19:03:06 PST 2017
WFM; added them to ExtParameterInfo in r296076. Thanks for the idea!
On Wed, Feb 15, 2017 at 5:44 PM, Richard Smith <richard at metafoo.co.uk>
wrote:
> On 15 February 2017 at 17:32, George Burgess IV <
> george.burgess.iv at gmail.com> wrote:
>
>> I remember that we wanted to pretend that pass_object_size isn't a part
>> of the FunctionType during the review that added it, though.
>>
>
> I remember we wanted to not add extra fake "parameters" to the
> FunctionType to model pass_object_size. I don't remember whether or why we
> wanted it to not be part of the function type at all -- on reflection, it
> seems as much a part of the type as, say, a calling convention (which it
> is, in some sense).
>
>
>> Do you think that would be better than serializing parameters before we
>> serialize template info? AFAICT, we only do merging after we start reading
>> the template info, so I can't immediately see why that wouldn't work.
>>
>
> I would be concerned about the possibility of that introducing dependency
> cycles into the deserialization process. For instance, merging default
> arguments for function parameters may require us to have already merged the
> function itself into its redeclaration chain (we don't currently model that
> quite correctly, so we probably won't hit it today).
>
>
>> On Wed, Feb 15, 2017 at 4:55 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>>
>>> On 15 February 2017 at 14:43, George Burgess IV via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>>
>>>> Author: gbiv
>>>> Date: Wed Feb 15 16:43:27 2017
>>>> New Revision: 295252
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=295252&view=rev
>>>> Log:
>>>> [Modules] Consider enable_if attrs in isSameEntity.
>>>>
>>>> Two functions that differ only in their enable_if attributes are
>>>> considered overloads, so we should check for those when we're trying to
>>>> figure out if two functions are mergeable.
>>>>
>>>> We need to do the same thing for pass_object_size, as well. Looks like
>>>> that'll be a bit less trivial, since we sometimes do these merging
>>>> checks before we have pass_object_size attributes available (see the
>>>> merge checks in ASTDeclReader::VisitFunctionDecl that happen before we
>>>> read parameters, and merge checks in calls to ReadDeclAs<>()).
>>>>
>>>
>>> Perhaps the best way to tackle this would be to track the presence of
>>> pass_object_size as part of the function type (in the ExtParameterInfo data
>>> on the function type).
>>>
>>> Added:
>>>> cfe/trunk/test/Modules/Inputs/overloadable-attrs/
>>>> cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h
>>>> cfe/trunk/test/Modules/Inputs/overloadable-attrs/module.modulemap
>>>> cfe/trunk/test/Modules/overloadable-attrs.cpp
>>>> Modified:
>>>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>>>
>>>> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serializat
>>>> ion/ASTReaderDecl.cpp?rev=295252&r1=295251&r2=295252&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
>>>> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Feb 15 16:43:27
>>>> 2017
>>>> @@ -2656,6 +2656,44 @@ static bool isSameTemplateParameterList(
>>>> return true;
>>>> }
>>>>
>>>> +/// Determine whether the attributes we can overload on are identical
>>>> for A and
>>>> +/// B. Expects A and B to (otherwise) have the same type.
>>>> +static bool hasSameOverloadableAttrs(const FunctionDecl *A,
>>>> + const FunctionDecl *B) {
>>>> + SmallVector<const EnableIfAttr *, 4> AEnableIfs;
>>>> + // Since this is an equality check, we can ignore that enable_if
>>>> attrs show up
>>>> + // in reverse order.
>>>> + for (const auto *EIA : A->specific_attrs<EnableIfAttr>())
>>>> + AEnableIfs.push_back(EIA);
>>>> +
>>>> + SmallVector<const EnableIfAttr *, 4> BEnableIfs;
>>>> + for (const auto *EIA : B->specific_attrs<EnableIfAttr>())
>>>> + BEnableIfs.push_back(EIA);
>>>> +
>>>> + // Two very common cases: either we have 0 enable_if attrs, or we
>>>> have an
>>>> + // unequal number of enable_if attrs.
>>>> + if (AEnableIfs.empty() && BEnableIfs.empty())
>>>> + return true;
>>>> +
>>>> + if (AEnableIfs.size() != BEnableIfs.size())
>>>> + return false;
>>>> +
>>>> + llvm::FoldingSetNodeID Cand1ID, Cand2ID;
>>>> + for (unsigned I = 0, E = AEnableIfs.size(); I != E; ++I) {
>>>> + Cand1ID.clear();
>>>> + Cand2ID.clear();
>>>> +
>>>> + AEnableIfs[I]->getCond()->Profile(Cand1ID, A->getASTContext(),
>>>> true);
>>>> + BEnableIfs[I]->getCond()->Profile(Cand2ID, B->getASTContext(),
>>>> true);
>>>> + if (Cand1ID != Cand2ID)
>>>> + return false;
>>>> + }
>>>> +
>>>> + // FIXME: This doesn't currently consider pass_object_size
>>>> attributes, since
>>>> + // we aren't guaranteed that A and B have valid parameter lists yet.
>>>> + return true;
>>>> +}
>>>> +
>>>> /// \brief Determine whether the two declarations refer to the same
>>>> entity.
>>>> static bool isSameEntity(NamedDecl *X, NamedDecl *Y) {
>>>> assert(X->getDeclName() == Y->getDeclName() && "Declaration name
>>>> mismatch!");
>>>> @@ -2711,8 +2749,10 @@ static bool isSameEntity(NamedDecl *X, N
>>>> CtorY->getInheritedConstructo
>>>> r().getConstructor()))
>>>> return false;
>>>> }
>>>> - return (FuncX->getLinkageInternal() ==
>>>> FuncY->getLinkageInternal()) &&
>>>> - FuncX->getASTContext().hasSameType(FuncX->getType(),
>>>> FuncY->getType());
>>>> + return FuncX->getLinkageInternal() == FuncY->getLinkageInternal()
>>>> &&
>>>> + FuncX->getASTContext().hasSameType(FuncX->getType(),
>>>> + FuncY->getType()) &&
>>>> + hasSameOverloadableAttrs(FuncX, FuncY);
>>>> }
>>>>
>>>> // Variables with the same type and linkage match.
>>>>
>>>> Added: cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>>>> nputs/overloadable-attrs/a.h?rev=295252&view=auto
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h (added)
>>>> +++ cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h Wed Feb 15
>>>> 16:43:27 2017
>>>> @@ -0,0 +1,16 @@
>>>> +namespace enable_if_attrs {
>>>> +constexpr int fn1() __attribute__((enable_if(0, ""))) { return 0; }
>>>> +constexpr int fn1() { return 1; }
>>>> +
>>>> +constexpr int fn2() { return 1; }
>>>> +constexpr int fn2() __attribute__((enable_if(0, ""))) { return 0; }
>>>> +
>>>> +constexpr int fn3(int i) __attribute__((enable_if(!i, ""))) { return
>>>> 0; }
>>>> +constexpr int fn3(int i) __attribute__((enable_if(i, ""))) { return 1;
>>>> }
>>>> +
>>>> +constexpr int fn4(int i) { return 0; }
>>>> +constexpr int fn4(int i) __attribute__((enable_if(i, ""))) { return 1;
>>>> }
>>>> +
>>>> +constexpr int fn5(int i) __attribute__((enable_if(i, ""))) { return 1;
>>>> }
>>>> +constexpr int fn5(int i) { return 0; }
>>>> +}
>>>>
>>>> Added: cfe/trunk/test/Modules/Inputs/overloadable-attrs/module.modu
>>>> lemap
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>>>> nputs/overloadable-attrs/module.modulemap?rev=295252&view=auto
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/test/Modules/Inputs/overloadable-attrs/module.modulemap
>>>> (added)
>>>> +++ cfe/trunk/test/Modules/Inputs/overloadable-attrs/module.modulemap
>>>> Wed Feb 15 16:43:27 2017
>>>> @@ -0,0 +1,3 @@
>>>> +module a {
>>>> + header "a.h"
>>>> +}
>>>>
>>>> Added: cfe/trunk/test/Modules/overloadable-attrs.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/o
>>>> verloadable-attrs.cpp?rev=295252&view=auto
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/test/Modules/overloadable-attrs.cpp (added)
>>>> +++ cfe/trunk/test/Modules/overloadable-attrs.cpp Wed Feb 15 16:43:27
>>>> 2017
>>>> @@ -0,0 +1,19 @@
>>>> +// RUN: rm -rf %t
>>>> +// RUN: %clang_cc1 -I%S/Inputs/overloadable-attrs -fmodules \
>>>> +// RUN: -fmodule-map-file=%S/Inputs/ov
>>>> erloadable-attrs/module.modulemap \
>>>> +// RUN: -fmodules-cache-path=%t -verify %s -std=c++11
>>>> +//
>>>> +// Ensures that we don't merge decls with attrs that we allow
>>>> overloading on.
>>>> +//
>>>> +// expected-no-diagnostics
>>>> +
>>>> +#include "a.h"
>>>> +
>>>> +static_assert(enable_if_attrs::fn1() == 1, "");
>>>> +static_assert(enable_if_attrs::fn2() == 1, "");
>>>> +static_assert(enable_if_attrs::fn3(0) == 0, "");
>>>> +static_assert(enable_if_attrs::fn3(1) == 1, "");
>>>> +static_assert(enable_if_attrs::fn4(0) == 0, "");
>>>> +static_assert(enable_if_attrs::fn4(1) == 1, "");
>>>> +static_assert(enable_if_attrs::fn5(0) == 0, "");
>>>> +static_assert(enable_if_attrs::fn5(1) == 1, "");
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org
>>>> http://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/20170223/f4903fc6/attachment.html>
More information about the cfe-commits
mailing list