[clang] Ignore template parameter visibility (PR #72092)

Fangrui Song via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 13 17:16:48 PST 2023


MaskRay wrote:

Thanks for the comment. Frankly, I am at a loss in deriving a criterion (for shouldConsiderTemplateVisibility) to select one of the following rules.

(1) factor in both template arguments and template parameters
(2) factor in only template arguments
(3) factor in nothing.

Could you elaborate? But, I feel that we don't need the template parameter complexity (I have debugged GCC and confirmed that it ignores template parameters).

Note that the lengthy comment "Visibility rules aren't rigorously externally specified, but here ..." does not mention template parameters, either.

---

> In the absence of visibility attributes, a template that's templated over e.g. an enum with hidden visibility seems like it ought to have hidden visibility.

Agree. This is `test63` in `clang/test/CodeGenCXX/visibility.cpp`.
```
enum HIDDEN E { E0 };
...
A::foo<E0>(); // hidden visibility w/o or w/o the patch. default visibility in GCC
```

GCC currently uses default visibility, which I believe is not reasonable (missing case in `decl2.cc:min_vis_expr_r`)
I pushed a commit to consider enum (TemplateArgument::Integral) for non-type template arguments.

```
@@ -268,10 +268,14 @@ LinkageComputer::getLVForTemplateArgumentList(ArrayRef<TemplateArgument> Args,
   for (const TemplateArgument &Arg : Args) {
     switch (Arg.getKind()) {
     case TemplateArgument::Null:
+      continue;
+
     case TemplateArgument::Integral:
+      LV.merge(getLVForType(*Arg.getIntegralType(), computation));
+      continue;
     case TemplateArgument::Expression:
+      LV.merge(getLVForType(*Arg.getAsExpr()->getType(), computation));
       continue;
```

I cannot figure out a test case for `TemplateArgument::Expression`. I wonder whether it applies to `X<&s.s> x5;` (address of static member), which Clang doesn't support.

After the change, does it seem more feasible to ignore template parameters?


https://github.com/llvm/llvm-project/pull/72092


More information about the cfe-commits mailing list