[PATCH] D141450: [Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default

Thu Jan 12 16:20:11 PST 2023

dblaikie added a comment.

In D141450#4049026 <https://reviews.llvm.org/D141450#4049026>, @Bigcheese wrote:

> In D141450#4044680 <https://reviews.llvm.org/D141450#4044680>, @dblaikie wrote:
>
>>> This is a problem because some existing ObjectiveC code is not compatible with LSV
>>
>> Hmm, how is that true? Does this code only build with Clang Header Modules enabled, and can't build without that? (if it could build without it, I don't know why it'd need LSV, if I'm understanding all these things correctly... )
>
> It only builds without modules or without LSV.

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).

My understanding was that LSV wouldn't reject anything that non-modular builds accepted, but would prevent modules from accepting things that non-modular builds rejects. (so LSV is a way to remove modular build false-accepts, basically)

Have I got that wrong in some way(s)?

> The code either relies on incidental header inclusion at the top level, or in other cases there are bugs with ObjC categories and protocols in LSV mode.

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).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141450