jansvoboda11 added a comment.
In D141450#4049049 <https://reviews.llvm.org/D141450#4049049>, @dblaikie wrote:
> Perhaps I've got things confused, but my understanding of LSV was that it
> prevented other headers in the same modulemap from leaking into the
> use/inclusion of one header in a module. But that indirect inclusions were
> still valid/exposed (eg: header A, B, and C in a module, A includes C -
> without LSV, including A also incidentally exposes B, but with or without LSV
> including A does expose C).
Regardless of LSV, importing A doesn't incidentally expose B. For exposing C,
module A would need to re-export C.
> My understanding was that LSV wouldn't reject anything that non-modular
> builds accepted,
LSV rejects things that both non-LSV modular and textual builds accept:
// RUN: rm -rf %t
// RUN: split-file %s %t
//--- module.modulemap
module M {
module S1 { header "s1.h" }
module S2 { header "s2.h" }
}
//--- s1.h
#define T1 int
//--- s2.h
T1 foo();
//--- tu.c
#include "s1.h"
#include "s2.h"
// Let's see how "T1" becomes available in "s2.h" with different
configurations.
// Textual build: succeeds, "tu.c" includes "s1.h" before "s2.h".
// RUN: %clang_cc1 -fsyntax-only %t/tu.c
// Modular build: succeeds, the entire compilation of M shares one
preprocessing context,
// which includes "s1.h" before "s2.h" due to their order in
the module map.
// RUN: %clang_cc1 -fsyntax-only %t/tu.c -fmodules -fimplicit-module-maps
-fmodules-cache-path=%t/cache
// Modular build with LSV: fails, compilation of M creates separate context
for each submodule,
// so "s2.h" does not see symbols of "s1.h" (without
explicitly importing it).
// RUN: %clang_cc1 -fsyntax-only %t/tu.c -fmodules -fimplicit-module-maps
-fmodules-cache-path=%t/cache -fmodules-local-submodule-visibility
> but would prevent modules from accepting things that non-modular builds
> rejects. (so LSV is a way to remove modular build false-accepts, basically)
This is true:
...
//--- tu.c
#include "s2.h"
// Textual build: fails, "s2.h" has no way of knowing about "T1".
// RUN: %clang_cc1 -fsyntax-only %t/tu.c
// Modular build: succeeds, "T1" is available in "s2.h" as a leftover from
compiling "s1.h".
// RUN: %clang_cc1 -fsyntax-only %t/tu.c -fmodules -fimplicit-module-maps
-fmodules-cache-path=%t/cache
// Modular build with LSV: fails, "T1" is not available in the preprocessing
context of "s2.h".
// RUN: %clang_cc1 -fsyntax-only %t/tu.c -fmodules -fimplicit-module-maps
-fmodules-cache-path=%t/cache -fmodules-local-submodule-visibility
> Bugs/incompleteness with LSV+ObjC I could believe/understand, and if that's
> the main issue I guess that's a choice that can be made about which bugs are
> worth fixing, etc, and maybe we should document the LSV change as working
> around those bugs for now (perhaps forever - if the benefits of LSV aren't
> worth the investment in fixing those bugs).
So yeah, we probably have some bugs/incompleteness specific to LSV+ObjC, but
our SDK also contains code that compiles fine with both textual inclusion and
modules, but fails with modules + LSV. Having a way to disable LSV under
Objective-C++20 is necessary for us in that case. Note that this patch doesn't
change any behavior yet, it just makes
`-fno-modules-local-submodule-visibility` available in `-cc1`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141450/new/
https://reviews.llvm.org/D141450
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
