diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -13,6 +13,8 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H +#include "Headers.h" +#include "index/Symbol.h" #include "index/SymbolID.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclObjC.h" @@ -118,6 +120,18 @@ /// return nullptr as protocols don't have an implementation. const ObjCImplDecl *getCorrespondingObjCImpl(const ObjCContainerDecl *D); +/// Infer the include directive to use for the given \p FileName. It aims for +/// #import for ObjC files and #include for the rest. +/// +/// - For source files we use LangOpts directly to infer ObjC-ness. +/// - For header files we also check for symbols declared by the file and +/// existing include directives, as the language can be set to ObjC++ as a +/// fallback in the absence of compile flags. +Symbol::IncludeDirective +preferredIncludeDirective(llvm::StringRef FileName, const LangOptions &LangOpts, + ArrayRef MainFileIncludes, + ArrayRef TopLevelDecls); + /// Returns a QualType as string. The result doesn't contain unwritten scopes /// like anonymous/inline namespace. std::string printType(const QualType QT, const DeclContext &CurContext, diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -376,6 +376,38 @@ return nullptr; } +Symbol::IncludeDirective +preferredIncludeDirective(llvm::StringRef FileName, const LangOptions &LangOpts, + ArrayRef MainFileIncludes, + 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 : 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 (const Decl *D : TopLevelDecls) + if (isa( + D)) + return Symbol::IncludeDirective::Import; + + return Symbol::IncludeDirective::Include; +} + std::string printType(const QualType QT, const DeclContext &CurContext, const llvm::StringRef Placeholder) { std::string Result; 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,6 +30,9 @@ /// 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); }; 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().MainFileIncludes, AST.getLocalTopLevelDecls()); const SourceManager &SM = AST.getSourceManager(); findExplicitReferences( AST.getASTContext(), diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -171,6 +171,10 @@ // If true, parse emplace-like functions in the preamble. bool PreambleParseForwardingFunctions = false; + /// Whether include fixer insertions for Objective-C code should use #import + /// instead of #include. + bool ImportInsertions = false; + explicit operator TUScheduler::Options() const; }; // Sensible default options for use in tests. @@ -434,6 +438,8 @@ bool PreambleParseForwardingFunctions = false; + bool ImportInsertions = false; + // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex) llvm::StringMap> CachedCompletionFuzzyFindRequestByFile; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -188,6 +188,7 @@ UseDirtyHeaders(Opts.UseDirtyHeaders), LineFoldingOnly(Opts.LineFoldingOnly), PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions), + ImportInsertions(Opts.ImportInsertions), WorkspaceRoot(Opts.WorkspaceRoot), Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate : TUScheduler::NoInvalidation), @@ -261,6 +262,7 @@ std::string ActualVersion = DraftMgr.addDraft(File, Version, Contents); ParseOptions Opts; Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions; + Opts.ImportInsertions = ImportInsertions; // Compile command is set asynchronously during update, as it can be slow. ParseInputs Inputs; 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 @@ -187,6 +187,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; @@ -277,9 +283,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 std::nullopt; } @@ -395,10 +401,10 @@ Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted)); }; bool ShouldInsert = C.headerToInsertIfAllowed(Opts).has_value(); + Symbol::IncludeDirective Directive = insertionDirective(Opts); // 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)) { @@ -406,7 +412,9 @@ Include.Header = ToInclude->first; if (ToInclude->second && ShouldInsert) Include.Insertion = Includes.insert( - ToInclude->first, tooling::IncludeDirective::Include); + ToInclude->first, Directive == Symbol::Import + ? tooling::IncludeDirective::Import + : tooling::IncludeDirective::Include); 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/Compiler.h b/clang-tools-extra/clangd/Compiler.h --- a/clang-tools-extra/clangd/Compiler.h +++ b/clang-tools-extra/clangd/Compiler.h @@ -40,6 +40,8 @@ // Options to run clang e.g. when parsing AST. struct ParseOptions { bool PreambleParseForwardingFunctions = false; + + bool ImportInsertions = false; }; /// Information required to run clang, e.g. to parse AST or do code completion. 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 @@ -320,8 +320,7 @@ llvm::StringSet<> InsertedHeaders; 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) { @@ -329,7 +328,9 @@ continue; if (auto Fix = insertHeader(ToInclude->first, (Sym.Scope + Sym.Name).str(), - tooling::IncludeDirective::Include)) + Directive == Symbol::Import + ? tooling::IncludeDirective::Import + : tooling::IncludeDirective::Include)) 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 @@ -86,6 +86,7 @@ /// 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 std::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" @@ -560,12 +562,21 @@ auto Inserter = std::make_shared( Filename, Inputs.Contents, Style, BuildDir.get(), &Clang->getPreprocessor().getHeaderSearchInfo()); + ArrayRef MainFileIncludes; if (Preamble) { + MainFileIncludes = Preamble->Includes.MainFileIncludes; 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 = + Inputs.Opts.ImportInsertions + ? preferredIncludeDirective(Filename, Clang->getLangOpts(), + MainFileIncludes, {}) + : Symbol::Include; 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); @@ -720,6 +731,10 @@ return LocalTopLevelDecls; } +llvm::ArrayRef ParsedAST::getLocalTopLevelDecls() const { + return LocalTopLevelDecls; +} + const MainFileMacros &ParsedAST::getMacros() const { return Macros; } const std::vector &ParsedAST::getMarks() const { return Marks; } diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -969,6 +969,7 @@ } Opts.UseDirtyHeaders = UseDirtyHeaders; Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions; + Opts.ImportInsertions = ImportInsertions; Opts.QueryDriverGlobs = std::move(QueryDriverGlobs); Opts.TweakFilter = [&](const Tweak &T) { if (T.hidden() && !HiddenFeatures) 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,6 +68,7 @@ Pair(sym("Y", index::SymbolKind::Variable, "@N@tar@S@X@FI@\\0").ID, 2), Pair(_ /*a*/, 3))); + EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Include); } } // namespace } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/ASTTests.cpp b/clang-tools-extra/clangd/unittests/ASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ASTTests.cpp @@ -11,6 +11,7 @@ #include "Annotations.h" #include "ParsedAST.h" #include "TestTU.h" +#include "index/Symbol.h" #include "clang/AST/ASTTypeTraits.h" #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" @@ -618,6 +619,45 @@ hasReservedScope(*findUnqualifiedDecl(AST, "secret").getDeclContext())); } +TEST(ClangdAST, PreferredIncludeDirective) { + auto ComputePreferredDirective = [](TestTU &TU) { + auto AST = TU.build(); + return preferredIncludeDirective(AST.tuPath(), AST.getLangOpts(), + AST.getIncludeStructure().MainFileIncludes, + AST.getLocalTopLevelDecls()); + }; + TestTU ObjCTU = TestTU::withCode(R"cpp( + int main() {} + )cpp"); + ObjCTU.Filename = "TestTU.m"; + EXPECT_EQ(ComputePreferredDirective(ObjCTU), + Symbol::IncludeDirective::Import); + + TestTU HeaderTU = TestTU::withCode(R"cpp( + #import "TestTU.h" + )cpp"); + HeaderTU.Filename = "TestTUHeader.h"; + HeaderTU.ExtraArgs = {"-xobjective-c++-header"}; + EXPECT_EQ(ComputePreferredDirective(HeaderTU), + Symbol::IncludeDirective::Import); + + // ObjC language option is not enough for headers. + HeaderTU.Code = R"cpp( + #include "TestTU.h" + )cpp"; + EXPECT_EQ(ComputePreferredDirective(HeaderTU), + Symbol::IncludeDirective::Include); + + HeaderTU.Code = R"cpp( + @interface Foo + @end + + Foo * getFoo(); + )cpp"; + EXPECT_EQ(ComputePreferredDirective(HeaderTU), + 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 @@ -2736,17 +2736,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;