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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
