<div dir="ltr">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. 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.<div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 15, 2017 at 4:55 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On 15 February 2017 at 14:43, George Burgess IV via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: gbiv<br>
Date: Wed Feb 15 16:43:27 2017<br>
New Revision: 295252<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=295252&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=295252&view=rev</a><br>
Log:<br>
[Modules] Consider enable_if attrs in isSameEntity.<br>
<br>
Two functions that differ only in their enable_if attributes are<br>
considered overloads, so we should check for those when we're trying to<br>
figure out if two functions are mergeable.<br>
<br>
We need to do the same thing for pass_object_size, as well. Looks like<br>
that'll be a bit less trivial, since we sometimes do these merging<br>
checks before we have pass_object_size attributes available (see the<br>
merge checks in ASTDeclReader::VisitFunctionDe<wbr>cl that happen before we<br>
read parameters, and merge checks in calls to ReadDeclAs<>()).<br></blockquote><div><br></div></span><div>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).</div><div><div class="m_5815907478191920849m_-2896096919112994645h5"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Added:<br>
cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/<br>
cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/a.h<br>
cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/module.modu<wbr>lemap<br>
cfe/trunk/test/Modules/overloa<wbr>dable-attrs.cpp<br>
Modified:<br>
cfe/trunk/lib/Serialization/AS<wbr>TReaderDecl.cpp<br>
<br>
Modified: cfe/trunk/lib/Serialization/AS<wbr>TReaderDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=295252&r1=295251&r2=295252&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Serializat<wbr>ion/ASTReaderDecl.cpp?rev=2952<wbr>52&r1=295251&r2=295252&view=di<wbr>ff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Serialization/AS<wbr>TReaderDecl.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/AS<wbr>TReaderDecl.cpp Wed Feb 15 16:43:27 2017<br>
@@ -2656,6 +2656,44 @@ static bool isSameTemplateParameterList(<br>
return true;<br>
}<br>
<br>
+/// Determine whether the attributes we can overload on are identical for A and<br>
+/// B. Expects A and B to (otherwise) have the same type.<br>
+static bool hasSameOverloadableAttrs(const FunctionDecl *A,<br>
+ const FunctionDecl *B) {<br>
+ SmallVector<const EnableIfAttr *, 4> AEnableIfs;<br>
+ // Since this is an equality check, we can ignore that enable_if attrs show up<br>
+ // in reverse order.<br>
+ for (const auto *EIA : A->specific_attrs<EnableIfAttr<wbr>>())<br>
+ AEnableIfs.push_back(EIA);<br>
+<br>
+ SmallVector<const EnableIfAttr *, 4> BEnableIfs;<br>
+ for (const auto *EIA : B->specific_attrs<EnableIfAttr<wbr>>())<br>
+ BEnableIfs.push_back(EIA);<br>
+<br>
+ // Two very common cases: either we have 0 enable_if attrs, or we have an<br>
+ // unequal number of enable_if attrs.<br>
+ if (AEnableIfs.empty() && BEnableIfs.empty())<br>
+ return true;<br>
+<br>
+ if (AEnableIfs.size() != BEnableIfs.size())<br>
+ return false;<br>
+<br>
+ llvm::FoldingSetNodeID Cand1ID, Cand2ID;<br>
+ for (unsigned I = 0, E = AEnableIfs.size(); I != E; ++I) {<br>
+ Cand1ID.clear();<br>
+ Cand2ID.clear();<br>
+<br>
+ AEnableIfs[I]->getCond()->Prof<wbr>ile(Cand1ID, A->getASTContext(), true);<br>
+ BEnableIfs[I]->getCond()->Prof<wbr>ile(Cand2ID, B->getASTContext(), true);<br>
+ if (Cand1ID != Cand2ID)<br>
+ return false;<br>
+ }<br>
+<br>
+ // FIXME: This doesn't currently consider pass_object_size attributes, since<br>
+ // we aren't guaranteed that A and B have valid parameter lists yet.<br>
+ return true;<br>
+}<br>
+<br>
/// \brief Determine whether the two declarations refer to the same entity.<br>
static bool isSameEntity(NamedDecl *X, NamedDecl *Y) {<br>
assert(X->getDeclName() == Y->getDeclName() && "Declaration name mismatch!");<br>
@@ -2711,8 +2749,10 @@ static bool isSameEntity(NamedDecl *X, N<br>
CtorY->getInheritedConstructo<wbr>r().getConstructor()))<br>
return false;<br>
}<br>
- return (FuncX->getLinkageInternal() == FuncY->getLinkageInternal()) &&<br>
- FuncX->getASTContext().hasSame<wbr>Type(FuncX->getType(), FuncY->getType());<br>
+ return FuncX->getLinkageInternal() == FuncY->getLinkageInternal() &&<br>
+ FuncX->getASTContext().hasSam<wbr>eType(FuncX->getType(),<br>
+ FuncY->getType()) &&<br>
+ hasSameOverloadableAttrs(Func<wbr>X, FuncY);<br>
}<br>
<br>
// Variables with the same type and linkage match.<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/a.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h?rev=295252&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Modules/I<wbr>nputs/overloadable-attrs/a.h?r<wbr>ev=295252&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/a.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/a.h Wed Feb 15 16:43:27 2017<br>
@@ -0,0 +1,16 @@<br>
+namespace enable_if_attrs {<br>
+constexpr int fn1() __attribute__((enable_if(0, ""))) { return 0; }<br>
+constexpr int fn1() { return 1; }<br>
+<br>
+constexpr int fn2() { return 1; }<br>
+constexpr int fn2() __attribute__((enable_if(0, ""))) { return 0; }<br>
+<br>
+constexpr int fn3(int i) __attribute__((enable_if(!i, ""))) { return 0; }<br>
+constexpr int fn3(int i) __attribute__((enable_if(i, ""))) { return 1; }<br>
+<br>
+constexpr int fn4(int i) { return 0; }<br>
+constexpr int fn4(int i) __attribute__((enable_if(i, ""))) { return 1; }<br>
+<br>
+constexpr int fn5(int i) __attribute__((enable_if(i, ""))) { return 1; }<br>
+constexpr int fn5(int i) { return 0; }<br>
+}<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/module.modu<wbr>lemap<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/overloadable-attrs/module.modulemap?rev=295252&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Modules/I<wbr>nputs/overloadable-attrs/modul<wbr>e.modulemap?rev=295252&view=au<wbr>to</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/module.modu<wbr>lemap (added)<br>
+++ cfe/trunk/test/Modules/Inputs/<wbr>overloadable-attrs/module.modu<wbr>lemap Wed Feb 15 16:43:27 2017<br>
@@ -0,0 +1,3 @@<br>
+module a {<br>
+ header "a.h"<br>
+}<br>
<br>
Added: cfe/trunk/test/Modules/overloa<wbr>dable-attrs.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/overloadable-attrs.cpp?rev=295252&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Modules/o<wbr>verloadable-attrs.cpp?rev=2952<wbr>52&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/overloa<wbr>dable-attrs.cpp (added)<br>
+++ cfe/trunk/test/Modules/overloa<wbr>dable-attrs.cpp Wed Feb 15 16:43:27 2017<br>
@@ -0,0 +1,19 @@<br>
+// RUN: rm -rf %t<br>
+// RUN: %clang_cc1 -I%S/Inputs/overloadable-attrs -fmodules \<br>
+// RUN: -fmodule-map-file=%S/Inputs/ov<wbr>erloadable-attrs/module.module<wbr>map \<br>
+// RUN: -fmodules-cache-path=%t -verify %s -std=c++11<br>
+//<br>
+// Ensures that we don't merge decls with attrs that we allow overloading on.<br>
+//<br>
+// expected-no-diagnostics<br>
+<br>
+#include "a.h"<br>
+<br>
+static_assert(enable_if_attrs<wbr>::fn1() == 1, "");<br>
+static_assert(enable_if_attrs<wbr>::fn2() == 1, "");<br>
+static_assert(enable_if_attrs<wbr>::fn3(0) == 0, "");<br>
+static_assert(enable_if_attrs<wbr>::fn3(1) == 1, "");<br>
+static_assert(enable_if_attrs<wbr>::fn4(0) == 0, "");<br>
+static_assert(enable_if_attrs<wbr>::fn4(1) == 1, "");<br>
+static_assert(enable_if_attrs<wbr>::fn5(0) == 0, "");<br>
+static_assert(enable_if_attrs<wbr>::fn5(1) == 1, "");<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>