ilya-biryukov wrote:

Here's a reproducer for our breakage:
```cpp
// RUN: rm -rf %t
// RUN: split-file %s %t
//
// First, build a module with a header.
//
// 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/modules1.map 
-fmodule-map-file=%t/modules2.map -fmodule-name=b -emit-module \
// RUN:   -fmodule-file=%t/a.pcm -xc++ -fmodules-embed-all-files -o %t/b.pcm 
%t/modules2.map
// 
// Remove the header and check we can still build the code that finds it in a 
PCM.
//
// RUN: rm %t/foo.h
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/modules2.map 
-fmodule-file=%t/b.pcm -fsyntax-only %t/use.cpp

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

//--- modules2.map
module all_textual {
  module foo {
    textual header "foo.h"
    export *
  }
  module wrap_foo {
    textual header "wrap_foo.h"
    export *
  }
  export *
}

module b {
  module wrap_foo {
    private header "wrap_foo.h"
    export *
  }
  export *
}


//--- foo.h
#ifndef FOO_H
#define FOO_H
void foo();
#endif

//--- wrap_foo.h
#include "foo.h"

//--- use.cpp
#include "wrap_foo.h"

void test() {
  foo();
}
```

It previously built fine because the `all_textual.foo` and its parent module 
used to be marked as unavailable because `foo.h` could not be found here: 
https://github.com/llvm/llvm-project/blob/09fd8f0093b8ff489d76285d893be152e4ca4c24/clang/lib/Lex/ModuleMap.cpp#L326

and therefore it made us pick the module `b.wrap_foo` later when resolving 
`wrap_foo.h` instead of `all_textual.wrap_foo` based on the code here: 
https://github.com/llvm/llvm-project/blob/09fd8f0093b8ff489d76285d893be152e4ca4c24/clang/lib/Lex/ModuleMap.cpp#L618

After the change, we no longer mark `all_textual` as unavailable and pick 
`all_textual.wrap_foo`, which causes us to reprocess the header.

I know what we do is cheesy and we might need to reconsider how to approach it, 
but this is a breaking change and it would be great to find a resolution for it 
before committing to the new behavior. Could we revert this while we discuss 
the potential ways out of it?

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