[PATCH] D118893: [C++20][Modules] Track valid import state.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1543
+def err_import_not_allowed_here : Error<
+  "imports must be contiguous and immediately follow the module declaration">;
+def err_import_in_global_fragment : Error<
----------------
urnathan wrote:
> iains wrote:
> > urnathan wrote:
> > > "imports must immediately follow a module declaration"?  (the contiguous isn't adding anything IMHO)
> > um, maybe I need two diagnostics then - the "contiguous" aspect applies when imports are split by a non-import statement.
> > 
> > Selecting between the two case on the basis of a flag seems unproductive here.  Would two diagnostics seem more reasonable to you?
> > 
> I don't follow.  If the imports are split by a non-import, then the latter ones don't immediately follow the module declaration.
The imports could exist in non-module unit. A example could be found at: http://eel.is/c++draft/basic.lookup.general#example-1. So the wording might not be accurate. I think we could refer to the wording in the standard [[ http://eel.is/c++draft/module.import#1 | [module.import]p1 ]]:

> In a module unit, all module-import-declarations and export-declarations exporting module-import-declarations shall appear before all other declarations in the declaration-seq of the translation-unit and of the private-module-fragment (if any).

If I don't misread this, I think it implies that imports could not be the first decl in non-module unit.


================
Comment at: clang/test/Modules/cxx20-import-diagnostics-a.cpp:125
+
+#else
+#error "no MODE set"
----------------
It should be allowed to import a module in non-module unit. For example:
```
import std;
int main() {
    std::cout << "Hello World.\n";
    return 0;
}
```

I guess we lack a test for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893