[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 14 02:53:37 PDT 2022
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
More information about the cfe-commits
mailing list