kbobyrev updated this revision to Diff 364709. kbobyrev added a comment. Improve comments, refactor some pieces, add more tests.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105426/new/ https://reviews.llvm.org/D105426 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/Headers.h clang-tools-extra/clangd/IWYU.cpp clang-tools-extra/clangd/IWYU.h clang-tools-extra/clangd/unittests/CMakeLists.txt clang-tools-extra/clangd/unittests/IWYUTests.cpp
Index: clang-tools-extra/clangd/unittests/IWYUTests.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/unittests/IWYUTests.cpp @@ -0,0 +1,127 @@ +//===--- IWYUTests.cpp ------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Annotations.h" +#include "IWYU.h" +#include "TestTU.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +using testing::ElementsAreArray; + +TEST(IWYU, ReferencedLocations) { + struct TestCase { + std::string HeaderCode; + std::string MainCode; + }; + TestCase Cases[] = { + { + "int ^x();", + "int y = x();", + }, + { + "class ^X;", + "X *y;", + }, + { + "using ^Integer = int;", + "Integer x;", + }, + { + "struct ^X{int ^a;}; X ^foo();", + "int y = foo().a;", + }, + { + "class ^X{}; X ^foo();", + "auto bar() { return foo(); }", + }, + { + "class ^X; class ^X{}; class ^X;", + "X *y;", + }, + { + "struct ^X { ^X(int) {} int ^foo(); };", + "auto x = X(42); auto y = x.foo();", + }, + { + "struct ^X { static bool ^foo(); }; bool X::^foo() {}", + "auto b = X::foo();", + }, + { + "template <typename> class ^X;", + "X<int> *y;", + }, + { + "typedef bool ^Y; template <typename T> struct ^X {};", + "X<Y> x;", + }, + { + "struct Foo; struct ^Foo{}; typedef Foo ^Bar;", + "Bar b;", + }, + { + "class ^X{}; X ^getX();", + "auto x = getX();", + }, + { + "namespace ns { struct ^X; struct ^X {}; }", + "using ns::X;", + }, + { + "namespace ns { struct X; struct X {}; }", + "using namespace ns;", + }, + { + // FIXME(kirillbobyrev): Should B also be marked as used? + "struct ^A {}; using B = A; using ^C = B;", + "C a;", + }, + { + "enum ^Color { ^Red = 42, Green = 9000};", + "int MyColor = Red;", + }, + { + "struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };", + "int Lang = X::CXX;", + }, + { + // FIXME(kirillbobyrev): Should report the UsingDecl. + // This information is not preserved in the AST. + // ^ + "namespace ns { class ^X; }; using ns::X;", + "X *y;", + }}; + for (const TestCase &T : Cases) { + TestTU TU; + TU.Code = T.MainCode; + Annotations Header(T.HeaderCode); + TU.HeaderCode = Header.code().str(); + auto AST = TU.build(); + + std::vector<Position> Points; + for (const auto &Loc : findReferencedLocations(AST)) { + if (AST.getSourceManager().getBufferName(Loc).endswith( + TU.HeaderFilename)) { + Points.push_back(offsetToPosition( + TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc))); + } + } + llvm::sort(Points); + + EXPECT_EQ(Points, Header.points()) << T.HeaderCode << "\n---\n" + << T.MainCode; + } +} + +} // namespace +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/unittests/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/unittests/CMakeLists.txt +++ clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -61,6 +61,7 @@ IndexActionTests.cpp IndexTests.cpp InlayHintTests.cpp + IWYUTests.cpp JSONTransportTests.cpp LoggerTests.cpp LSPBinderTests.cpp Index: clang-tools-extra/clangd/IWYU.h =================================================================== --- /dev/null +++ clang-tools-extra/clangd/IWYU.h @@ -0,0 +1,55 @@ +//===--- IWYU.h - include-what-you-use analysis -----------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This defines functionality modeled after Include-What-You-Use +/// (https://include-what-you-use.org/). The variant we implement is not +/// equivalent to the original tool since we have different design limitations +/// such as performance requirements but we are trying to make it compatible +/// with the original tool in the most popular and useful cases. +/// +/// FIXME(kirillbobyrev): Add support for IWYU pragmas. +/// FIXME(kirillbobyrev): Add support for mapping files. +/// FIXME(kirillbobyrev): Add support for standard library headers. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_IWYU_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_IWYU_H + +#include "Headers.h" +#include "ParsedAST.h" +#include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/DenseSet.h" + +namespace clang { +namespace clangd { + +using ReferencedLocations = llvm::DenseSet<SourceLocation>; +/// Goes through main file AST and computes all the locations used symbols are +/// coming from. These locations are later used to determine which headers +/// should be marked as "used" and "directly used". +/// +/// Because of the clangd limitations, this should be efficient, +/// hence there is only one AST traversal for the whole process. +/// +/// NOTE: The balance is currently shifted towards considering some of the +/// implicitly used symbols as "used" for the sake of providing less noisy +/// "unused header" diagnostics. However, this should be re-evalated once we +/// also want to surface "missing header" diagnostics. +ReferencedLocations findReferencedLocations(ParsedAST &AST); + +/// Computes all referenced files given a set of SourceLocations that were +/// extracted from symbol declarations. +llvm::DenseSet<FileID> findReferencedFiles(const ReferencedLocations &Locs, + const SourceManager &SM); + +} // namespace clangd +} // namespace clang + +#endif Index: clang-tools-extra/clangd/IWYU.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/IWYU.cpp @@ -0,0 +1,158 @@ +//===--- IWYU.cpp -----------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "IWYU.h" +#include "support/Logger.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Basic/SourceLocation.h" + +namespace clang { +namespace clangd { +namespace { + +/// Main "driver" of the IWYU symbol usage discovery mechanism. Crawler +/// traverses the AST and feeds in the locations of (sometimes implicitly) used +/// symbols into \p Result. +class ReferencedLocationCrawler + : public RecursiveASTVisitor<ReferencedLocationCrawler> { +public: + ReferencedLocationCrawler(ReferencedLocations &Result) : Result(Result) {} + + bool VisitDeclRefExpr(DeclRefExpr *DRE) { + add(DRE->getDecl()); + add(DRE->getFoundDecl()); + return true; + } + + bool VisitMemberExpr(MemberExpr *ME) { + add(ME->getMemberDecl()); + add(ME->getFoundDecl().getDecl()); + return true; + } + + bool VisitTagType(TagType *TT) { + add(TT->getDecl()); + return true; + } + + bool VisitCXXConstructExpr(CXXConstructExpr *CCE) { + add(CCE->getConstructor()); + return true; + } + + bool VisitTemplateSpecializationType(TemplateSpecializationType *TST) { + if (shouldAdd(TST)) { + add(TST->getTemplateName().getAsTemplateDecl()); // Primary template. + add(TST->getAsCXXRecordDecl()); // Specialization + } + return true; + } + + bool VisitTypedefType(TypedefType *TT) { + add(TT->getDecl()); + return true; + } + + // Consider types of any subexpression used, even if the type is not named. + // This is helpful in getFoo().bar(), where Foo must be complete. + // FIXME(kirillbobyrev): This is a tricky case that raises a question - should + // we consider implicit types to be "used"? With auto's it's not so clear. + // This should probably be controlled with a flag. + bool VisitExpr(Expr *E) { + TraverseType(E->getType()); + return true; + } + + bool TraverseType(QualType T) { + if (shouldAdd(T.getTypePtrOrNull())) { // don't care about quals + RecursiveASTVisitor<ReferencedLocationCrawler>::TraverseType(T); + } + return true; + } + + bool VisitUsingDecl(UsingDecl *D) { + for (const auto *Shadow : D->shadows()) { + add(Shadow->getTargetDecl()); + } + return true; + } + +private: + void add(const Decl *D) { + if (!D || !shouldAdd(D->getCanonicalDecl())) { + return; + } + for (const Decl *Redecl : D->redecls()) { + Result.insert(Redecl->getLocation()); + } + } + + bool shouldAdd(const void *P) { return P && Visited.insert(P).second; } + + ReferencedLocations &Result; + llvm::DenseSet<const void *> Visited; +}; + +// Given a set of referenced FileIDs, determines all the potentially-referenced +// files and macros by traversing expansion/spelling locations of macro IDs. +// This is used to map the referenced SourceLocations onto real files. +struct ReferencedFiles { + ReferencedFiles(const SourceManager &SM) : SM(SM) {} + llvm::DenseSet<FileID> Files; + llvm::DenseSet<FileID> Macros; + const SourceManager &SM; + + void add(SourceLocation Loc) { add(SM.getFileID(Loc), Loc); } + + void add(FileID FID, SourceLocation Loc) { + if (FID.isInvalid()) + return; + assert(SM.isInFileID(Loc, FID)); + if (Loc.isFileID()) { + Files.insert(FID); + return; + } + // Don't process the same macro FID twice. + if (!Macros.insert(FID).second) + return; + const auto &Exp = SM.getSLocEntry(FID).getExpansion(); + add(Exp.getSpellingLoc()); + add(Exp.getExpansionLocStart()); + add(Exp.getExpansionLocEnd()); + } +}; + +} // namespace + +ReferencedLocations findReferencedLocations(ParsedAST &AST) { + ReferencedLocations Result; + ReferencedLocationCrawler Crawler(Result); + Crawler.TraverseAST(AST.getASTContext()); + // FIXME(kirillbobyrev): Handle macros. + return Result; +} + +llvm::DenseSet<FileID> findReferencedFiles(const ReferencedLocations &Locs, + const SourceManager &SM) { + std::vector<SourceLocation> Sorted{Locs.begin(), Locs.end()}; + llvm::sort(Sorted); // This groups by FileID! + ReferencedFiles Result(SM); + for (auto It = Sorted.begin(); It < Sorted.end();) { + auto FID = SM.getFileID(*It); + Result.add(FID, *It); + // Cheaply skip over all the other locations from the same FileID. + // This avoids lots of redundant Loc->File lookups for the same file. + do { + ++It; + } while (It != Sorted.end() && SM.isInFileID(*It, FID)); + } + return std::move(Result.Files); +} + +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/Headers.h =================================================================== --- clang-tools-extra/clangd/Headers.h +++ clang-tools-extra/clangd/Headers.h @@ -129,6 +129,8 @@ llvm::StringRef IncludedName, llvm::StringRef IncludedRealName); + using AbstractIncludeGraph = llvm::DenseMap<unsigned, SmallVector<unsigned>>; + private: // Identifying files in a way that persists from preamble build to subsequent // builds is surprisingly hard. FileID is unavailable in InclusionDirective(), Index: clang-tools-extra/clangd/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/CMakeLists.txt +++ clang-tools-extra/clangd/CMakeLists.txt @@ -85,6 +85,7 @@ Hover.cpp IncludeFixer.cpp InlayHints.cpp + IWYU.cpp JSONTransport.cpp PathMapping.cpp Protocol.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
