aaron.ballman marked 3 inline comments as done.
aaron.ballman added a comment.

In D136953#3893039 <https://reviews.llvm.org/D136953#3893039>, @tschuett wrote:

> Are malformed imports an issue or are they already covered? There is limit 
> test coverage for import. Am I missing something?
>
>   import import;
>   import module;

It's unclear to me whether the constraints also apply to module imports or not. 
The wording for the constraints is in the section about module units and the 
section about module imports doesn't say anything further. Certainly we want to 
allow `import std;` instead of telling the user about the use of a reserved 
identifier, but it's less clear to me about `import module;` -- if the user 
writes that, they'll either get an error about not being able to find a module 
by that name, or they'd import the module (but how did they produce that module 
in the first place?).



================
Comment at: clang/docs/ReleaseNotes.rst:347
   Fixes `Issue 57562 <https://github.com/llvm/llvm-project/issues/57562>`_.
+- Clang now diagnoses use of invalid or reserved module names. Both are
+  diagnosed as an error, but the diagnostic is suppressed for use of reserved
----------------
erichkeane wrote:
> So question: ARE we diagnosing all 'use' of invalid/reserved module names?  
> So the idea is you cannot import:
> `std.concepts`?  Or are we diagnosing `export` only?
Export only for the moment. I'll update the release note accordingly.


================
Comment at: clang/lib/Sema/SemaModule.cpp:148
+/// Tests whether the given identifier is reserved as a module name and
+/// diagnoses if it is. Returs true if a diagnostic is emitted and false
+/// otherwise.
----------------
tschuett wrote:
> Returns
Good catch!


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

https://reviews.llvm.org/D136953

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

Reply via email to