[PATCH] D143906: [include-cleaner] Better support ambiguous std symbols

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:174
                                            const PragmaIncludes *PI) {
+  if (auto Headers = specialStandardSymbols(S); !Headers.empty())
+    return Headers;
----------------
kadircet wrote:
> rather than doing this here and bailing out early, i think we should just augment the `Headers` below, after `locateSymbol` and `findHeaders` is run.
> 
> that way we'll make sure rest of the logic applies in the future without special casing (e.g. we've some fixmes for treating implementation locations for stdlib symbols as providers, in such a scenario this early exits would create non-uniformity).
> it also implies we'll need to think about hints to attach here, which seems consistent with rest of the logic again (as we still assign hints to headers derived from stdlib recognizer).
> 
> so what about an alternative with a signature like: `llvm::SmallVector<Hinted<Header>> headersForSpecialSymbols(S, SM, PI);` then we can just initialize Headers with a call to it?
> i think we should just augment the Headers below, after locateSymbol and findHeaders is run. 

I'm not sure about it. Looks like this is subtle, it creates discrepancies for standard symbols, these special std symbols will have the underlying header as alternative while other normal std-recognizer symbols not. I think it would be better to be consistent for all std symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143906