[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:64
 
+using SymbolLocation = std::variant<SourceLocation, tooling::stdlib::Symbol>;
+/// A set of locations that provides the declaration, while indicating if
----------------
This is an important public API concept ==> It should be documented and part of a public header, I think


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:167
 
+std::vector<std::pair<SymbolLocation, bool /*IsDefinition*/>>
+locateDecl(const NamedDecl &ND) {
----------------
I don't think "definition" is the right concept here, the better signal in experiments was "is this declaration usable in general", which a function forward declaration **is**.
(In the prototype this was `IsComplete` and only set for template/class cases where completeness matters, `InIncomplete` is probably a bit cleaner.)

Also it would be nice to avoid passing bools around to represent subtle concepts (which definition is).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135953