rjmccall added inline comments.

================
Comment at: clang/lib/AST/Decl.cpp:380
+                                                ->getTemplatedDecl()
+                                                ->hasAttr<VisibilityAttr>();
 }
----------------
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.


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

Reply via email to