[PATCH] D53847: [C++2a] P0634r3: Down with typename!

Fri Sep 23 16:29:04 PDT 2022

ayzhao added inline comments.

================
Comment at: clang/lib/Sema/SemaDecl.cpp:367
+        // with this method returning a non-null ParsedType?
+        if (isa<ExportDecl>(CurContext))
+          return nullptr;
----------------
erichkeane wrote:
> Hmm... this scares me quite a bit, I don't really know what other fallout results from this, or other code that would hit here.
> 
> We should probably spend more time making sure we understand this better.
Following up with my [previous comment](https://reviews.llvm.org/D53847?id=459205#inline-1291007) about the failing test that this hack was supposed to fix:

I added the `typename` keyword to the `export template` statement and lo and behold, Clang emits the same "declaration does not declare anything" diagnostic instead of the namespace scope diagnostic [0].

To me, this feels like that the [original test case was wrong](https://github.com/llvm/llvm-project/blob/a707675dbba9ca3ec6e668f86fea2240a85ca171/clang/test/CXX/module/module.interface/p2-2.cpp#L18):

1. There should've been a `typename` keyword in the `export` statement. Perhaps the author of the test wasn't aware that the `typename` keyword was required, or perhaps the author already assumed that P0634r3 was already implemented in clang.
1. The test currently passing (with Clang emitting the namespace diagnostic instead of the declaration diagnostic) seems to be a freak coincidence.

I'm not familiar at all with C++ modules - is this a bug with exporting types?

[0]: https://godbolt.org/z/PYvh68Tq7

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53847/new/

https://reviews.llvm.org/D53847