[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Aaron Ballman via Phabricator via cfe-commits]

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