dgoldman created this revision.
Herald added subscribers: kadircet, arphaman, javed.absar.
Herald added a project: All.
dgoldman requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
These are still disabled by default, but will work in ObjC code if you
enable the `-import-insertions` flag.
This requires ASTSignals to be available; before ASTSignals are
available, we will always use #include. Once they are available, the
behavior varies as follows:
- For source files, use #import if the ObjC language flag is enabled
- For header files:
- If the ObjC language flag is disabled, use #include
- If the header file contains any #imports, use #import
- If the header file references any ObjC decls, use #import
- Otherwise, use #include
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139458
Files:
clang-tools-extra/clangd/ASTSignals.cpp
clang-tools-extra/clangd/ASTSignals.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/IncludeFixer.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -18,6 +18,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
#include "../TidyProvider.h"
+#include "ASTSignals.h"
#include "Compiler.h"
#include "FeatureModule.h"
#include "ParsedAST.h"
@@ -72,6 +73,9 @@
// Parse options pass on to the ParseInputs
ParseOptions ParseOpts = {};
+ // ASTSignals to use. Must be manually set.
+ std::shared_ptr<const ASTSignals> Signals;
+
// Whether to use overlay the TestFS over the real filesystem. This is
// required for use of implicit modules.where the module file is written to
// disk and later read back.
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -127,7 +127,7 @@
/*StoreInMemory=*/true,
/*PreambleCallback=*/nullptr);
auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI),
- Diags.take(), Preamble);
+ Diags.take(), Preamble, Signals);
if (!AST) {
llvm::errs() << "Failed to build code:\n" << Code;
std::abort();
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -206,7 +206,7 @@
return std::nullopt;
}
return ParsedAST::build(testPath(TU.Filename), TU.inputs(FS), std::move(CI),
- {}, BaselinePreamble);
+ {}, BaselinePreamble, {});
}
std::string getPreamblePatch(llvm::StringRef Baseline,
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -505,7 +505,7 @@
TU.Code = ModifiedContents.str();
Inputs = TU.inputs(FS);
auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
- {}, EmptyPreamble);
+ {}, EmptyPreamble, {});
ASSERT_TRUE(PatchedAST);
ASSERT_FALSE(PatchedAST->getDiagnostics());
@@ -550,7 +550,7 @@
TU.Code = "";
Inputs = TU.inputs(FS);
auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
- {}, BaselinePreamble);
+ {}, BaselinePreamble, {});
ASSERT_TRUE(PatchedAST);
// Ensure source location information is correct.
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -223,7 +223,8 @@
log("Building AST...");
AST = ParsedAST::build(File, Inputs, std::move(Invocation),
- /*InvocationDiags=*/std::vector<Diag>{}, Preamble);
+ /*InvocationDiags=*/std::vector<Diag>{}, Preamble,
+ /*Signals=*/{});
if (!AST) {
elog("Failed to build AST");
return false;
@@ -287,7 +288,8 @@
IgnoringDiagConsumer IgnoreDiags;
auto Invocation = buildCompilerInvocation(Inputs, IgnoreDiags);
Duration Val = Time([&] {
- ParsedAST::build(File, Inputs, std::move(Invocation), {}, Preamble);
+ ParsedAST::build(File, Inputs, std::move(Invocation), {}, Preamble,
+ /*Signals=*/{});
});
vlog(" Measured {0} ==> {1}", Checks, Val);
return Val;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -979,9 +979,12 @@
// return a compatible preamble as ASTWorker::update blocks.
llvm::Optional<ParsedAST> NewAST;
if (Invocation) {
+ std::shared_ptr<const ASTSignals> Signals;
+ std::shared_ptr<const PreambleData> Preamble =
+ getPossiblyStalePreamble(&Signals);
NewAST = ParsedAST::build(FileName, FileInputs, std::move(Invocation),
CompilerInvocationDiagConsumer.take(),
- getPossiblyStalePreamble());
+ Preamble, Signals);
++ASTBuildCount;
}
AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
@@ -1188,8 +1191,11 @@
IdleASTs.take(this, &ASTAccessForDiag);
if (!AST || !InputsAreLatest) {
auto RebuildStartTime = DebouncePolicy::clock::now();
- llvm::Optional<ParsedAST> NewAST = ParsedAST::build(
- FileName, Inputs, std::move(Invocation), CIDiags, *LatestPreamble);
+ std::shared_ptr<const ASTSignals> Signals;
+ getPossiblyStalePreamble(&Signals);
+ llvm::Optional<ParsedAST> NewAST =
+ ParsedAST::build(FileName, Inputs, std::move(Invocation), CIDiags,
+ *LatestPreamble, Signals);
auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
++ASTBuildCount;
// Try to record the AST-build time, to inform future update debouncing.
Index: clang-tools-extra/clangd/ParsedAST.h
===================================================================
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -40,6 +40,7 @@
namespace clang {
class Sema;
namespace clangd {
+struct ASTSignals;
class HeuristicResolver;
/// Stores and provides access to parsed AST.
@@ -52,7 +53,8 @@
build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::unique_ptr<clang::CompilerInvocation> CI,
llvm::ArrayRef<Diag> CompilerInvocationDiags,
- std::shared_ptr<const PreambleData> Preamble);
+ std::shared_ptr<const PreambleData> Preamble,
+ std::shared_ptr<const ASTSignals> Signals);
ParsedAST(ParsedAST &&Other);
ParsedAST &operator=(ParsedAST &&Other);
@@ -125,7 +127,8 @@
std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens,
- MainFileMacros Macros, std::vector<PragmaMark> Marks,
+ std::shared_ptr<const ASTSignals> Signals, MainFileMacros Macros,
+ std::vector<PragmaMark> Marks,
std::vector<Decl *> LocalTopLevelDecls,
llvm::Optional<std::vector<Diag>> Diags, IncludeStructure Includes,
CanonicalIncludes CanonIncludes);
@@ -148,6 +151,8 @@
/// - Does not have spelled or expanded tokens for files from preamble.
syntax::TokenBuffer Tokens;
+ /// Signals for the main file, if available.
+ std::shared_ptr<const ASTSignals> Signals;
/// All macro definitions and expansions in the main file.
MainFileMacros Macros;
// Pragma marks in the main file.
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -11,6 +11,7 @@
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
#include "../clang-tidy/ClangTidyModuleRegistry.h"
#include "AST.h"
+#include "ASTSignals.h"
#include "Compiler.h"
#include "Config.h"
#include "Diagnostics.h"
@@ -25,6 +26,7 @@
#include "TidyProvider.h"
#include "index/CanonicalIncludes.h"
#include "index/Index.h"
+#include "index/Symbol.h"
#include "support/Logger.h"
#include "support/Trace.h"
#include "clang/AST/ASTContext.h"
@@ -342,7 +344,8 @@
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::unique_ptr<clang::CompilerInvocation> CI,
llvm::ArrayRef<Diag> CompilerInvocationDiags,
- std::shared_ptr<const PreambleData> Preamble) {
+ std::shared_ptr<const PreambleData> Preamble,
+ std::shared_ptr<const ASTSignals> Signals) {
trace::Span Tracer("BuildAST");
SPAN_ATTACH(Tracer, "File", Filename);
@@ -563,8 +566,11 @@
for (const auto &Inc : Preamble->Includes.MainFileIncludes)
Inserter->addExisting(Inc);
}
+ Symbol::IncludeDirective Directive = Symbol::IncludeDirective::Include;
+ if (Signals)
+ Directive = Signals->InsertionDirective;
FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
- /*IndexRequestLimit=*/5);
+ /*IndexRequestLimit=*/5, Directive);
ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl,
const clang::Diagnostic &Info) {
return FixIncludes->fix(DiagLevl, Info);
@@ -669,9 +675,9 @@
}
ParsedAST Result(Filename, Inputs.Version, std::move(Preamble),
std::move(Clang), std::move(Action), std::move(Tokens),
- std::move(Macros), std::move(Marks), std::move(ParsedDecls),
- std::move(Diags), std::move(Includes),
- std::move(CanonIncludes));
+ std::move(Signals), std::move(Macros), std::move(Marks),
+ std::move(ParsedDecls), std::move(Diags),
+ std::move(Includes), std::move(CanonIncludes));
if (Result.Diags) {
auto UnusedHeadersDiags =
issueUnusedIncludesDiagnostics(Result, Inputs.Contents);
@@ -766,15 +772,17 @@
std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action,
- syntax::TokenBuffer Tokens, MainFileMacros Macros,
- std::vector<PragmaMark> Marks,
+ syntax::TokenBuffer Tokens,
+ std::shared_ptr<const ASTSignals> Signals,
+ MainFileMacros Macros, std::vector<PragmaMark> Marks,
std::vector<Decl *> LocalTopLevelDecls,
llvm::Optional<std::vector<Diag>> Diags,
IncludeStructure Includes, CanonicalIncludes CanonIncludes)
: TUPath(TUPath), Version(Version), Preamble(std::move(Preamble)),
Clang(std::move(Clang)), Action(std::move(Action)),
- Tokens(std::move(Tokens)), Macros(std::move(Macros)),
- Marks(std::move(Marks)), Diags(std::move(Diags)),
+ Tokens(std::move(Tokens)), Signals(std::move(Signals)),
+ Macros(std::move(Macros)), Marks(std::move(Marks)),
+ Diags(std::move(Diags)),
LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
Resolver = std::make_unique<HeuristicResolver>(getASTContext());
Index: clang-tools-extra/clangd/IncludeFixer.h
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.h
+++ clang-tools-extra/clangd/IncludeFixer.h
@@ -34,9 +34,10 @@
class IncludeFixer {
public:
IncludeFixer(llvm::StringRef File, std::shared_ptr<IncludeInserter> Inserter,
- const SymbolIndex &Index, unsigned IndexRequestLimit)
+ const SymbolIndex &Index, unsigned IndexRequestLimit,
+ Symbol::IncludeDirective Directive)
: File(File), Inserter(std::move(Inserter)), Index(Index),
- IndexRequestLimit(IndexRequestLimit) {}
+ IndexRequestLimit(IndexRequestLimit), Directive(Directive) {}
/// Returns include insertions that can potentially recover the diagnostic.
/// If Info is a note and fixes are returned, they should *replace* the note.
@@ -80,6 +81,7 @@
const SymbolIndex &Index;
const unsigned IndexRequestLimit; // Make at most 5 index requests.
mutable unsigned IndexRequestCount = 0;
+ const Symbol::IncludeDirective Directive;
// These collect the last unresolved name so that we can associate it with the
// diagnostic.
Index: clang-tools-extra/clangd/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.cpp
+++ clang-tools-extra/clangd/IncludeFixer.cpp
@@ -319,10 +319,11 @@
// different scopes from the same header, but this case should be rare and is
// thus ignored.
llvm::StringSet<> InsertedHeaders;
+ auto InsertDirective = Directive == Symbol::IncludeDirective::Import ?
+ tooling::IncludeDirective::Import : tooling::IncludeDirective::Include;
for (const auto &Sym : Syms) {
for (const auto &Inc : getRankedIncludes(Sym)) {
- // FIXME: We should support #import directives here.
- if ((Inc.Directive & clang::clangd::Symbol::Include) == 0)
+ if ((Inc.Directive & Directive) == 0)
continue;
if (auto ToInclude = Inserted(Sym, Inc.Header)) {
if (ToInclude->second) {
@@ -330,7 +331,7 @@
continue;
if (auto Fix =
insertHeader(ToInclude->first, (Sym.Scope + Sym.Name).str(),
- tooling::IncludeDirective::Include))
+ InsertDirective))
Fixes.push_back(std::move(*Fix));
}
} else {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -177,6 +177,12 @@
return Result;
}
+Symbol::IncludeDirective insertionDirective(const CodeCompleteOptions &Opts) {
+ if (!Opts.ImportInsertions || !Opts.MainFileSignals)
+ return Symbol::IncludeDirective::Include;
+ return Opts.MainFileSignals->InsertionDirective;
+}
+
// Identifier code completion result.
struct RawIdentifier {
llvm::StringRef Name;
@@ -267,9 +273,9 @@
if (SM.isInMainFile(SM.getExpansionLoc(RD->getBeginLoc())))
return std::nullopt;
}
+ Symbol::IncludeDirective Directive = insertionDirective(Opts);
for (const auto &Inc : RankedIncludeHeaders)
- // FIXME: We should support #import directives here.
- if ((Inc.Directive & clang::clangd::Symbol::Include) != 0)
+ if ((Inc.Directive & Directive) != 0)
return Inc.Header;
return None;
}
@@ -385,18 +391,21 @@
Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
};
bool ShouldInsert = C.headerToInsertIfAllowed(Opts).has_value();
+ Symbol::IncludeDirective Directive = insertionDirective(Opts);
+ tooling::IncludeDirective InsertDirective =
+ Directive == Symbol::Import ? tooling::IncludeDirective::Import
+ : tooling::IncludeDirective::Include;
// Calculate include paths and edits for all possible headers.
for (const auto &Inc : C.RankedIncludeHeaders) {
- // FIXME: We should support #import directives here.
- if ((Inc.Directive & clang::clangd::Symbol::Include) == 0)
+ if ((Inc.Directive & Directive) == 0)
continue;
if (auto ToInclude = Inserted(Inc.Header)) {
CodeCompletion::IncludeCandidate Include;
Include.Header = ToInclude->first;
if (ToInclude->second && ShouldInsert)
- Include.Insertion = Includes.insert(
- ToInclude->first, tooling::IncludeDirective::Include);
+ Include.Insertion =
+ Includes.insert(ToInclude->first, InsertDirective);
Completion.Includes.push_back(std::move(Include));
} else
log("Failed to generate include insertion edits for adding header "
Index: clang-tools-extra/clangd/ASTSignals.h
===================================================================
--- clang-tools-extra/clangd/ASTSignals.h
+++ clang-tools-extra/clangd/ASTSignals.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_ASTSIGNALS_H
#include "ParsedAST.h"
+#include "index/Symbol.h"
#include "index/SymbolID.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringMap.h"
@@ -29,6 +30,9 @@
/// Namespaces whose symbols are used in the file, and the number of such
/// distinct symbols.
llvm::StringMap<unsigned> RelatedNamespaces;
+ /// Whether include insertion should suggest imports or includes.
+ Symbol::IncludeDirective InsertionDirective =
+ Symbol::IncludeDirective::Include;
static ASTSignals derive(const ParsedAST &AST);
};
Index: clang-tools-extra/clangd/ASTSignals.cpp
===================================================================
--- clang-tools-extra/clangd/ASTSignals.cpp
+++ clang-tools-extra/clangd/ASTSignals.cpp
@@ -10,12 +10,37 @@
#include "AST.h"
#include "FindTarget.h"
#include "support/Trace.h"
+#include "clang/AST/DeclObjC.h"
namespace clang {
namespace clangd {
ASTSignals ASTSignals::derive(const ParsedAST &AST) {
trace::Span Span("ASTSignals::derive");
ASTSignals Signals;
+ bool CheckForObjC = false;
+ // Header files:
+ // - If ObjC language flag is disabled, use #include
+ // - If the header file contains any #imports, use #import
+ // - If any referenced decls in the header file are ObjC, use #import
+ // - Otherwise, use #include
+ //
+ // Source files: Use #import if the ObjC language flag is enabled
+ if (isHeaderFile(AST.tuPath(), AST.getLangOpts())) {
+ if (AST.getLangOpts().ObjC) {
+ CheckForObjC = true;
+ for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
+ if (MFI.Directive == tok::pp_import) {
+ Signals.InsertionDirective = Symbol::IncludeDirective::Import;
+ CheckForObjC = false;
+ break;
+ }
+ }
+ }
+ } else {
+ Signals.InsertionDirective = AST.getLangOpts().ObjC
+ ? Symbol::IncludeDirective::Import
+ : Symbol::IncludeDirective::Include;
+ }
const SourceManager &SM = AST.getSourceManager();
findExplicitReferences(
AST.getASTContext(),
@@ -38,6 +63,11 @@
if (!NS.empty())
Signals.RelatedNamespaces[NS]++;
}
+ if (CheckForObjC && isa<ObjCContainerDecl, ObjCIvarDecl,
+ ObjCMethodDecl, ObjCPropertyDecl>(ND)) {
+ Signals.InsertionDirective = Symbol::IncludeDirective::Import;
+ CheckForObjC = false;
+ }
}
},
AST.getHeuristicResolver());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
