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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to