cjdb added a comment.

In D105439#2874805 <https://reviews.llvm.org/D105439#2874805>, @cjdb wrote:

> In D105439#2874733 <https://reviews.llvm.org/D105439#2874733>, @lebedev.ri 
> wrote:
>
>> In D105439#2874706 <https://reviews.llvm.org/D105439#2874706>, @ldionne 
>> wrote:
>>
>>> I'm not entirely sure I understand the purpose of this patch. So the idea 
>>> is that let's say a tool suggests including `<__algorithm/find.h>` to get 
>>> the definition of `std::find` as a IWYU fix-it sort of suggestion, the user 
>>> would naively do that, and then the compiler (with this patch) would error 
>>> out saying "woops, you can't include that libc++ detail header". Is that 
>>> the idea?
>>>
>>> If that's it, then I would much rather fix the tools that incorrectly 
>>> suggest including those implementation detail headers in the first place. 
>>> Users will be less confused and we won't have to special-case a special 
>>> directory name, which I can imagine could cause issues. I think it's great 
>>> to try and give the proper diagnostic to users, but I think the correct 
>>> place to do that is in the tool that suggests it in the first place. 
>>> Thoughts?
>>
>> Regardless of whether this is about enhacing IWYU QoL or completely 
>> preventing inclusion of separate sub-headers,
>> it seems like that should be done in libc++ itself, it already has the means 
>> for that.
>
> I've already pointed out that it can't be done in libc++. The proposed header 
> alternative doesn't translate to Clang modules, and private modules tank 
> build times.

After a bit more experimenting (I was benchmarking to provide evidence), it 
seems as though private headers impact the first run of `llvm-lit` on libc++ 
tests, but not subsequent ones. Since this patch is user-oriented, I'm now in 
agreement that it //can// be punted back to the libc++ space. Thanks 
@lebedev.ri and apologies for my initial misleading assertion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105439

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

Reply via email to