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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to