[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.


================
Comment at: clang/lib/Sema/SemaLookup.cpp:1947
+  // DeclModule if it isn't (transitively) imported.
+  if (DeclModule->getTopLevelModule()->isModuleInterfaceUnit())
+    return true;
----------------
ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > iains wrote:
> > > > I think isModuleInterfaceUnit needs to include implementation partition units, since they also have a BMI  (is `isInterfaceOrPartition()` the right thing to use here?
> > > I think we couldn't use `isInterfaceOrPartition()` here. The call for `isModuleInterfaceUnit()` here is sufficient. For example, we could find the example at [[ https://eel.is/c++draft/module.reach#example-1 | [module.reach]example 1 ]], here in Translation unit #5:, the definition of struct B is not reachable since the definition lives in an implementation unit. (We could make it valid by making all additional TU as reachable)
> > > 
> > > Also the module interface unit shouldn't include implementation partition unit according to the wording: [[ https://eel.is/c++draft/module.unit#2 | [module.unit]p2 ]]. I agree that it is confusing since implementation partition unit is importable. But this is not our fault.
> > 
> > OK, perhaps I am missing something - just to clarify,...
> > 
> > In this (which I believe is legal like [module.unit] ex 1 TU 4.
> > ```
> > module;
> > ....
> > module Main;
> > 
> > import :ImplUnit; // this has a BMI which could contain reachable definitions, right?
> > 
> > ...
> > ```
> > 
> > 
> > 
> > 
> Yeah, I believe this is legal according to [module.reach]p1:
> > A translation unit U is necessarily reachable from a point P if U is a module interface unit on which the translation unit containing P has an interface dependency, **or the translation unit containing P imports U**, in either case prior to P.
> 
> Since module Main imports `:ImplUnit` directly, the  `:ImplUnit` TU is necessarily reachable.
(sorry for multiple iterations - I am trying to see if I missed some point ... )

... it seems to me that in valid code  `:ImplUnit` can have `Kind =` 
`ModulePartitionInterface`
or
`ModulePartitionImplementation`

the second is the special case of an implementation that provides a BMI also.




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

https://reviews.llvm.org/D113545