[PATCH] D120306: [clangd] IncludeCleaner: Add support for IWYU pragma private

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 22 03:39:48 PST 2022


sammccall added a comment.

In D120306#3337288 <https://reviews.llvm.org/D120306#3337288>, @kbobyrev wrote:

> In D120306#3337212 <https://reviews.llvm.org/D120306#3337212>, @sammccall wrote:
>
>> What's the policy this patch intends to implement?
>>
>> I'm a little concerned by building up maps of filenames with segment limits - it looks like some kind of heuristic/partial matching.
>
> Basically, handling the mappings that. IWYU pragmas provide.

Yes, I'm asking you to describe that handling in a couple of sentences so I can understand what you intend it to be.

e.g. for includecleaner in general, the approach is: walk the AST, map each node to decls it uses, map decls to locations, map locations to files, then check which includes point to files that were not identified.

> Are you concerned about the limit itself or the way I'm trying to find these headers in general? I was afraid of consuming too much memory, hence the limit; it's not crucial to the implementation, though, I can remove it if your concern is the limit rather than the approach.

Mostly I'm unclear on:

- whether we're comparing filenames among a small (e.g. the include stack - heuristics OK) or a large set (need to be very precise)
- whether the partial matching is semantically important or a performance optimitzation
- whether the concept of "real name" is significant or likely to cause problems
- whether storing another copy of the names of all the transitive headers is actually necessary for what you're trying to do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120306



More information about the cfe-commits mailing list