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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D136953#3892578 <https://reviews.llvm.org/D136953#3892578>, @aaron.ballman wrote:

> I think the standards wording here is a bit unclear as to what's intended and I had misunderstood the wording around which part of the path needs to be checked. Specifically:
>
> `All module-names either beginning with an identifier consisting of std followed by zero or more digits or containing a reserved identifier ([lex.name]) are reserved and shall not be specified in a module-declaration; no diagnostic is required.`
>
> I took this to mean all identifier components of a module name should check for std followed by zero or more digits, but I now believe it means only the first component in the path and it intends to only catch use of `std[0-9]+` and not `.*std[0-9]+.*`. e.g., `std.foo` is reserved while `foo.std` and `std12Three` are not. I've changed the patch accordingly while addressing the other review feedback.

Oh, yeah, the wording here looks confusing. And I agree with your understanding.



================
Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+      (FirstComponentName == "std" ||
----------------
std modules should be irreverent with system headers; The intuition of the wording should be that the users can't declare modules like `std` or `std.compat` to avoid possible conflicting. The approach I imaged may be add a new compilation flags (call it `-fstd-modules`) now. And if the compiler found a `std` module declaration without `-fstd-modules`, emit an error.  

For now, I think we can skip the check for `-fstd-modules` and add it back when we starts to support std modules actually.


================
Comment at: clang/test/Modules/reserved-names-1.cpp:23-24
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module std;    // expected-error {{'std' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module std0;   // expected-error {{'std0' is a reserved name for a module}} \
----------------
or diagnose about `export module std.foo`.


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

https://reviews.llvm.org/D136953