MaskRay added a comment.
In D153835#4478700 <https://reviews.llvm.org/D153835#4478700>, @efriedma wrote:
> Can you write the complete rule we're trying to follow here? Like, if you
> have a free function template, prioritize attributes in the following order
> ..., if you have a member function, use the following order ..., etc. I know
> that's a significant amount of writing, but I don't really have any intuition
> here about how it should work. So I'm not sure how to review this without a
> reference. (The closest thing I have to intuition is comparing the rules to
> the ones for C++ anonymous namespaces, but visibility is a lot more
> complicated.)
The rules are quite complex and sorry that I am unable to describe them...
> Given how complicated the rules are here, it might be easier to understand if
> you teach the relevant code in clang/lib/AST/Decl.cpp
> (getExplicitVisibilityAux, I think?) to explicitly check for attributes on
> the original template, instead of cloning the attribute.
If we don't clone `VisibilityAttr`, the best I can come up with is the
following patch
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -375,6 +375,9 @@ static bool shouldConsiderTemplateVisibility(const
FunctionDecl *fn,
if (!specInfo->isExplicitInstantiationOrSpecialization())
return true;
+ if (specInfo->getTemplate()->getTemplatedDecl()->hasAttr<VisibilityAttr>())
+ return false;
+
return !fn->hasAttr<VisibilityAttr>();
}
test71 still fails (while this patch fixes test71). Here is my observation:
As confirmed by `-fsyntax-only -Xclang -ast-dump`, the explicit instantiation
declaration (`extern template struct DEFAULT foo<int>;`) clones the
`FunctionTemplateDecl` tree (including `CXXMethodDecl`) without cloning
`VisibilityAttr`. This causes all `hasAttr<VisibilityAttr>()` code to behave
incorrectly like the status quo.
I think this `VisibilityAttr` cloning patch matches GCC's spirit better and
will enable simplification of the complex template instantiation/specialization
rules.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153835/new/
https://reviews.llvm.org/D153835
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
