ChuanqiXu added inline comments.

================
Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+      (FirstComponentName == "std" ||
----------------
aaron.ballman wrote:
> ChuanqiXu wrote:
> > cor3ntin wrote:
> > > ChuanqiXu wrote:
> > > > 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.
> > > The idea is that standard modules are built from system directories... it 
> > > seems a better heuristic than adding a flag for the purpose of 1 
> > > diagnostics ( maybe some other system library could in theory export std 
> > > with no warning, but I'm not super worried about that being a concern in 
> > > practice)
> > > The idea is that standard modules are built from system directories...
> > 
> > This is not true. For example, if someday libc++ supports std modules, then 
> > we need to build the std modules in our working directory, which is not 
> > system directories. And **ideally**, we would distribute and install module 
> > file in the system directories. But it is irreverent with the path of the 
> > source file.
> > then we need to build the std modules in our working directory, which is 
> > not system directories.
> 
> `-isystem`, pragmas, and linemarkers are all ways around that -- I don't 
> think we need a feature flag for this, unless I'm misunderstanding something.
Although it may be a little bit nit picker, the module unit which declares the 
std modules won't be header. It is a module unit. So it requires we implement 
std modules by wrapping linemarkers around the std modules declaration, which 
looks a little bit overkill.

And another point is that maybe we need to introduce another feature flags to 
implement std modules. Although I tried to implement std modules within the 
current features, I can't prove we can implement std modules in that way in the 
end of the day.

Let me add some more words. The standards require us to implement std modules 
without deprecating the system headers. This strategy leads the direction to 
"implement the components in the original headers and control their visibility 
in the std module unit".
It may look like:

```
//--- std.cppm
module;
#include <algorithm>
...
export module std;
```

Then how can control the visibility?  In my original experiments, I use the 
style:

```
//--- std.cppm
module;
#include <algorithm>
...
export module std;
export namespace std {
    using std::sort;
}
```

but this doesn't always work. For example, we can't `using` a in-class friend 
definition. And there are more reasons, the unreachable declarations in the 
global module fragment (the section from `module;` to `export module 
[module_name]`) can be discarded to reduce the size of the module file. And the 
reachable rules are complex. But the simple story is that it is highly possible 
the a lot of necessary declarations in global module fragment in the above 
snippet would be discarded so that the user can't use std modules correctly. I 
mean, we **may** need a special feature flag for it. And the method with 
`system headers` looks not good and semantics are not so consistency.




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