MaskRay added inline comments.
================ Comment at: clang/lib/AST/Decl.cpp:380 + ->getTemplatedDecl() + ->hasAttr<VisibilityAttr>(); } ---------------- rjmccall wrote: > Okay, this change seems wrong. A visibility attribute on an explicit > specialization or instantiation should definitely override everything else. > A visibility attribute on a template pattern should only override other > visibility information that we would derive from that pattern, like the > visibility of the template parameters; it should not override visibility from > the template arguments. You probably need this function to return an enum > with three cases: (1) factor in both template arguments and template > parameters, (2) factor in only template arguments, and (3) factor in nothing. > > Also, the bug here doesn't seem to be specific to function templates, and you > should fix the code for all three paths (function templates, class templates, > and variable templates). Basically we're universally mishandling visibility > attributes on template patterns for templates with non-type template > parameters with hidden visibility. I have added test coverage to `test51` in `visibility.cpp` and studied GCC's behavior. Sent https://github.com/llvm/llvm-project/pull/72092 to ignore visibility effects from template parameters. It will bring our behavior closer to GCC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154774/new/ https://reviews.llvm.org/D154774 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
