WFM; added them to ExtParameterInfo in r296076. Thanks for the idea! On Wed, Feb 15, 2017 at 5:44 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> On 15 February 2017 at 17:32, George Burgess IV < > george.burgess...@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 <rich...@metafoo.co.uk> >> wrote: >> >>> On 15 February 2017 at 14:43, George Burgess IV via cfe-commits < >>> cfe-commits@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@lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> >>> >>> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits