kbobyrev updated this revision to Diff 382358.
kbobyrev added a comment.
Add an elaborate comment on potential future improvements.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112447/new/
https://reviews.llvm.org/D112447
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -116,6 +116,26 @@
"struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };",
"int Lang = X::CXX;",
},
+ // Macros
+ {
+ "#define ^CONSTANT 42",
+ "int Foo = CONSTANT;",
+ },
+ {
+ "#define ^FOO x",
+ "#define BAR FOO",
+ },
+ {
+ "#define INNER 42\n"
+ "#define ^OUTER INNER",
+ "int answer = OUTER;",
+ },
+ {
+ "#define ^ANSWER 42\n"
+ "#define ^SQUARE(X) X * X",
+ "int sq = SQUARE(ANSWER);",
+ },
+ // Misc
{
"enum class ^Color : int;",
"enum class Color : int {};",
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -33,11 +33,13 @@
using ReferencedLocations = llvm::DenseSet<SourceLocation>;
/// Finds locations of all symbols used in the main file.
///
-/// Uses RecursiveASTVisitor to go through main file AST and computes all the
-/// locations used symbols are coming from. Returned locations may be macro
-/// expansions, and are not resolved to their spelling/expansion location. These
-/// locations are later used to determine which headers should be marked as
-/// "used" and "directly used".
+/// - For non-macros uses RecursiveASTVisitor to go through main file AST and
+/// computes all the locations used symbols are coming from. Returned
+/// locations may be macro expansions, and are not resolved to their spelling
+/// or expansion location. These locations are later used to determine which
+/// headers should / be marked as "used" and "directly used".
+/// - For macros iteratates over \p AST tokens and collects locations of the
+/// used macro definition.
///
/// We use this to compute unused headers, so we:
///
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,8 +11,11 @@
#include "Protocol.h"
#include "SourceCode.h"
#include "support/Logger.h"
+#include "support/Trace.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
@@ -168,13 +171,42 @@
return Result;
}
+// Gets the tokens from the main file, iterates through them and adds definition
+// locations for the found macros.
+void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
+ trace::Span Tracer("IncludeCleaner::findReferencedMacros");
+ auto &SM = AST.getSourceManager();
+ auto &PP = AST.getPreprocessor();
+ // FIXME(kirillbobyrev): We are iterating through all tokens in the main file.
+ // To improve performance, we can improve CollectMainFileMacros: right now
+ // it populates ParsedAST's MainFileMacros with macro references but does not
+ // process macro references in the definitions, e.g.
+ //
+ // #define FOO [BAR]
+ //
+ // Fixing this will improve other features relying on the index because the
+ // macro references will be seen even from other macro bodies.
+ for (const syntax::Token &Tok :
+ AST.getTokens().spelledTokens(SM.getMainFileID())) {
+ if (Tok.kind() != tok::identifier ||
+ !PP.getIdentifierInfo(Tok.text(SM))->hadMacroDefinition())
+ continue;
+ auto Macro = locateMacroAt(Tok, PP);
+ if (!Macro)
+ continue;
+ auto Loc = Macro->Info->getDefinitionLoc();
+ if (Loc.isValid())
+ Result.insert(Loc);
+ }
+}
+
} // namespace
ReferencedLocations findReferencedLocations(ParsedAST &AST) {
ReferencedLocations Result;
ReferencedLocationCrawler Crawler(Result);
Crawler.TraverseAST(AST.getASTContext());
- // FIXME(kirillbobyrev): Handle macros.
+ findReferencedMacros(AST, Result);
return Result;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
