vsapsai wrote:

> Thanks a lot, I am sorry for misreading or misunderstanding your comments 
> too. I hope we can put this behind ourselves.

Appreciate the understanding.

> [skip the technical description]

Thanks for the explanation, it gives a lot of food for thought. Still building 
a mental model of how `-fmodules-embed-all-files` and private headers work [and 
how they should work]. For example, it was surprising to me that

```
// RUN: rm -rf %t
// RUN: split-file %s %t
//
// Build modules 'a' and 'b' embedding all files.
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/modules1.map -fmodule-name=a 
-emit-module -xc++ -fmodules-embed-all-files -o %t/a.pcm %t/modules1.map
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/modules2.map -fmodule-name=b 
-emit-module -xc++ -fmodules-embed-all-files -o %t/b.pcm %t/modules2.map
//
// Try to access foo.h from a module.
// RUN: rm %t/foo.h
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/modules1.map 
-fmodule-file=%t/a.pcm -fsyntax-only %t/use.cpp

//--- modules1.map
module a {
  header "foo.h"
  export *
}

//--- modules2.map
module b {
  private header "foo.h"
  export *
}

//--- foo.h
void foo(void);

//--- use.cpp
#include "foo.h"
void test() {
  foo();
}
```

still cannot find "foo.h" despite `-fmodules-embed-all-files`.

> ## Looping back to #141792.
> (I am not sure if this was already addressed, sorry if I missed the 
> comments). How does the idea of always preferring to load a header from PCM 
> sound to folks? Given that access checking of headers and module resolution 
> are independent in Clang right now and given that same header in multiple 
> modules is rare, it seems like a behavior change we could potentially agree 
> on.
> 
> That being said, I also wouldn't be surprised if folks don't like this, it's 
> certainly not the cleanest design one could imagine. I would want to hear 
> opinions on that approach.

So far we are still aiming for a more principled solution. For example, if 
support of a header in multiple modules is a supported feature, then some kind 
of a spec with a more comprehensive testing would be appropriate. It is up to 
you but personally I feel like that's a lot of work and it is more pragmatic to 
skirt the issue and make sure your use case keeps working with smaller 
investments.

Personally, at the moment I'm interested in how `-fmodules-embed-all-files` 
works and if its behavior is consistent. Though it is possible I like putting 
stuff there because we don't rely on this flag and it feels less risky. Anyway, 
the first impression of using `-fmodules-embed-all-files` makes me suspicious 
it works inconsistently.

https://github.com/llvm/llvm-project/pull/138227
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to