diff --git a/clang-tools-extra/clangd/ASTSignals.h b/clang-tools-extra/clangd/ASTSignals.h --- a/clang-tools-extra/clangd/ASTSignals.h +++ b/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,8 +30,15 @@ /// Namespaces whose symbols are used in the file, and the number of such /// distinct symbols. llvm::StringMap RelatedNamespaces; + /// Preferred preprocessor directive to use for inclusions by the file. + Symbol::IncludeDirective InsertionDirective = + Symbol::IncludeDirective::Include; static ASTSignals derive(const ParsedAST &AST); + + static Symbol::IncludeDirective preferredIncludeDirective( + llvm::StringRef Filename, const LangOptions &LangOpts, + const IncludeStructure &Includes, ArrayRef TopLevelDecls); }; } // namespace clangd diff --git a/clang-tools-extra/clangd/ASTSignals.cpp b/clang-tools-extra/clangd/ASTSignals.cpp --- a/clang-tools-extra/clangd/ASTSignals.cpp +++ b/clang-tools-extra/clangd/ASTSignals.cpp @@ -9,13 +9,18 @@ #include "ASTSignals.h" #include "AST.h" #include "FindTarget.h" +#include "Headers.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; + Signals.InsertionDirective = preferredIncludeDirective( + AST.tuPath(), AST.getLangOpts(), AST.getIncludeStructure(), + AST.getLocalTopLevelDecls()); const SourceManager &SM = AST.getSourceManager(); findExplicitReferences( AST.getASTContext(), @@ -43,5 +48,36 @@ AST.getHeuristicResolver()); return Signals; } + +Symbol::IncludeDirective ASTSignals::preferredIncludeDirective( + llvm::StringRef Filename, const LangOptions &LangOpts, + const IncludeStructure &Includes, ArrayRef TopLevelDecls) { + // Always prefer #include for non-ObjC code. + if (!LangOpts.ObjC) + return Symbol::IncludeDirective::Include; + // If this is not a header file and has ObjC set as the language, prefer + // #import. + if (!isHeaderFile(Filename, LangOpts)) + return Symbol::IncludeDirective::Import; + + // Headers lack proper compile flags most of the time, so we might treat a + // header as ObjC accidentally. Perform some extra checks to make sure this + // works. + + // Any file with a #import, should keep #import-ing. + for (auto &Inc : Includes.MainFileIncludes) + if (Inc.Directive == tok::pp_import) + return Symbol::IncludeDirective::Import; + + // Any file declaring an ObjC decl should also be #import-ing. + // No need to look over the references, as the file doesn't have any #imports, + // it must be declaring interesting ObjC-like decls. + for (Decl *D : TopLevelDecls) + if (isa( + D)) + return Symbol::IncludeDirective::Import; + + return Symbol::IncludeDirective::Include; +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/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 " diff --git a/clang-tools-extra/clangd/IncludeFixer.h b/clang-tools-extra/clangd/IncludeFixer.h --- a/clang-tools-extra/clangd/IncludeFixer.h +++ b/clang-tools-extra/clangd/IncludeFixer.h @@ -34,9 +34,10 @@ class IncludeFixer { public: IncludeFixer(llvm::StringRef File, std::shared_ptr 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. diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp --- a/clang-tools-extra/clangd/IncludeFixer.cpp +++ b/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 { diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -85,7 +85,7 @@ /// This function returns top-level decls present in the main file of the AST. /// The result does not include the decls that come from the preamble. /// (These should be const, but RecursiveASTVisitor requires Decl*). - ArrayRef getLocalTopLevelDecls(); + ArrayRef getLocalTopLevelDecls() const; const llvm::Optional> &getDiagnostics() const { return Diags; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/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" @@ -559,12 +561,19 @@ auto Inserter = std::make_shared( Filename, Inputs.Contents, Style, BuildDir.get(), &Clang->getPreprocessor().getHeaderSearchInfo()); + IncludeStructure Includes; if (Preamble) { + Includes = Preamble->Includes; for (const auto &Inc : Preamble->Includes.MainFileIncludes) Inserter->addExisting(Inc); } + // FIXME: Consider piping through ASTSignals to fetch this to handle the + // case where a header file contains ObjC decls but no #imports. + Symbol::IncludeDirective Directive = + ASTSignals::preferredIncludeDirective(Filename, Clang->getLangOpts(), + Includes, {}); 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); @@ -715,7 +724,7 @@ return Clang->getPreprocessor(); } -llvm::ArrayRef ParsedAST::getLocalTopLevelDecls() { +llvm::ArrayRef ParsedAST::getLocalTopLevelDecls() const { return LocalTopLevelDecls; } diff --git a/clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp b/clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp --- a/clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp +++ b/clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp @@ -68,7 +68,44 @@ Pair(sym("Y", index::SymbolKind::Variable, "@N@tar@S@X@FI@\\0").ID, 2), Pair(_ /*a*/, 3))); + EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Include); } + +TEST(ASTSignals, DeriveInsertionDirective) { + TestTU ObjCTU = TestTU::withCode(R"cpp( + int main() {} + )cpp"); + ObjCTU.Filename = "TestTU.m"; + ASTSignals Signals = ASTSignals::derive(ObjCTU.build()); + EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Import); + + TestTU HeaderTU = TestTU::withCode(R"cpp( + #import "TestTU.h" + )cpp"); + HeaderTU.Filename = "TestTUHeader.h"; + HeaderTU.ExtraArgs = {"-xobjective-c++-header"}; + Signals = ASTSignals::derive(HeaderTU.build()); + EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Import); + + // ObjC language option is not enough for headers. + HeaderTU.Code = R"cpp( + #include "TestTU.h" + )cpp"; + Signals = ASTSignals::derive(HeaderTU.build()); + EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Include); + + HeaderTU.Code = R"cpp( + #include "TestTU.h" + + @interface Foo + @end + + Foo * getFoo(); + )cpp"; + Signals = ASTSignals::derive(HeaderTU.build()); + EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Import); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -2718,17 +2718,21 @@ Sym.CanonicalDeclaration.FileURI = DeclFile.c_str(); Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000, Symbol::Include | Symbol::Import); - - auto Results = completions("Fun^", {Sym}).Completions; + CodeCompleteOptions Opts; + // Should only take effect in import contexts. + Opts.ImportInsertions = true; + auto Results = completions("Fun^", {Sym}, Opts).Completions; assert(!Results.empty()); EXPECT_THAT(Results[0], AllOf(named("Func"), insertIncludeText("#include \"bar.h\"\n"))); - Results = completions("Fun^", {Sym}, {}, "Foo.m").Completions; + ASTSignals Signals; + Signals.InsertionDirective = Symbol::IncludeDirective::Import; + Opts.MainFileSignals = &Signals; + Results = completions("Fun^", {Sym}, Opts, "Foo.m").Completions; assert(!Results.empty()); - // TODO: Once #import integration support is done this should be #import. EXPECT_THAT(Results[0], - AllOf(named("Func"), insertIncludeText("#include \"bar.h\"\n"))); + AllOf(named("Func"), insertIncludeText("#import \"bar.h\"\n"))); Sym.IncludeHeaders[0].SupportedDirectives = Symbol::Import; Results = completions("Fun^", {Sym}).Completions;