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 @@ -191,7 +191,7 @@ const CodeCompletionResult *SemaResult = nullptr; const Symbol *IndexResult = nullptr; const RawIdentifier *IdentifierResult = nullptr; - llvm::SmallVector RankedIncludeHeaders; + llvm::SmallVector RankedIncludeHeaders; // Returns a token identifying the overload set this is part of. // 0 indicates it's not part of any overload set. @@ -267,7 +267,11 @@ if (SM.isInMainFile(SM.getExpansionLoc(RD->getBeginLoc()))) return std::nullopt; } - return RankedIncludeHeaders[0]; + for (const auto &Inc : RankedIncludeHeaders) + // FIXME: We should support #import directives here. + if ((Inc.Directive & clang::clangd::Symbol::Include) != 0) + return Inc.Header; + return None; } using Bundle = llvm::SmallVector; @@ -383,7 +387,11 @@ bool ShouldInsert = C.headerToInsertIfAllowed(Opts).has_value(); // Calculate include paths and edits for all possible headers. for (const auto &Inc : C.RankedIncludeHeaders) { - if (auto ToInclude = Inserted(Inc)) { + // FIXME: We should support #import directives here. + if ((Inc.Directive & clang::clangd::Symbol::Include) == 0) + continue; + + if (auto ToInclude = Inserted(Inc.Header)) { CodeCompletion::IncludeCandidate Include; Include.Header = ToInclude->first; if (ToInclude->second && ShouldInsert) @@ -392,7 +400,7 @@ } else log("Failed to generate include insertion edits for adding header " "(FileURI='{0}', IncludeHeader='{1}') into {2}: {3}", - C.IndexResult->CanonicalDeclaration.FileURI, Inc, FileName, + C.IndexResult->CanonicalDeclaration.FileURI, Inc.Header, FileName, ToInclude.takeError()); } // Prefer includes that do not need edits (i.e. already exist). diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h --- a/clang-tools-extra/clangd/Headers.h +++ b/clang-tools-extra/clangd/Headers.h @@ -45,6 +45,15 @@ bool valid() const; }; +/// A header and directives as stored in a Symbol. +struct SymbolInclude { + /// The header to include. This is either a URI or a verbatim include which is + /// quoted with <> or "". + llvm::StringRef Header; + /// The include directive to use, e.g. #import or #include. + Symbol::IncludeDirective Directive; +}; + /// Creates a `HeaderFile` from \p Header which can be either a URI or a literal /// include. llvm::Expected toHeaderFile(llvm::StringRef Header, @@ -52,7 +61,7 @@ // Returns include headers for \p Sym sorted by popularity. If two headers are // equally popular, prefer the shorter one. -llvm::SmallVector getRankedIncludes(const Symbol &Sym); +llvm::SmallVector getRankedIncludes(const Symbol &Sym); // An #include directive that we found in the main file. struct Inclusion { diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -211,7 +211,7 @@ return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; } -llvm::SmallVector getRankedIncludes(const Symbol &Sym) { +llvm::SmallVector getRankedIncludes(const Symbol &Sym) { auto Includes = Sym.IncludeHeaders; // Sort in descending order by reference count and header length. llvm::sort(Includes, [](const Symbol::IncludeHeaderWithReferences &LHS, @@ -220,9 +220,9 @@ return LHS.IncludeHeader.size() < RHS.IncludeHeader.size(); return LHS.References > RHS.References; }); - llvm::SmallVector Headers; + llvm::SmallVector Headers; for (const auto &Include : Includes) - Headers.push_back(Include.IncludeHeader); + Headers.push_back({Include.IncludeHeader, Include.supportedDirectives()}); return Headers; } 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 @@ -317,7 +317,10 @@ llvm::StringSet<> InsertedHeaders; for (const auto &Sym : Syms) { for (const auto &Inc : getRankedIncludes(Sym)) { - if (auto ToInclude = Inserted(Sym, Inc)) { + // FIXME: We should support #import directives here. + if ((Inc.Directive & clang::clangd::Symbol::Include) == 0) + continue; + if (auto ToInclude = Inserted(Sym, Inc.Header)) { if (ToInclude->second) { if (!InsertedHeaders.try_emplace(ToInclude->first).second) continue; @@ -326,8 +329,8 @@ Fixes.push_back(std::move(*Fix)); } } else { - vlog("Failed to calculate include insertion for {0} into {1}: {2}", Inc, - File, ToInclude.takeError()); + vlog("Failed to calculate include insertion for {0} into {1}: {2}", + Inc.Header, File, ToInclude.takeError()); } } } diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -248,11 +248,13 @@ if (SI.IncludeHeader == OI.IncludeHeader) { Found = true; SI.References += OI.References; + SI.SupportedDirectives |= OI.SupportedDirectives; break; } } if (!Found && MergeIncludes) - S.IncludeHeaders.emplace_back(OI.IncludeHeader, OI.References); + S.IncludeHeaders.emplace_back(OI.IncludeHeader, OI.References, + OI.supportedDirectives()); } S.Origin |= O.Origin | SymbolOrigin::Merge; diff --git a/clang-tools-extra/clangd/index/Serialization.cpp b/clang-tools-extra/clangd/index/Serialization.cpp --- a/clang-tools-extra/clangd/index/Serialization.cpp +++ b/clang-tools-extra/clangd/index/Serialization.cpp @@ -331,7 +331,7 @@ auto WriteInclude = [&](const Symbol::IncludeHeaderWithReferences &Include) { writeVar(Strings.index(Include.IncludeHeader), OS); - writeVar(Include.References, OS); + writeVar((Include.References << 2) | Include.SupportedDirectives, OS); }; writeVar(Sym.IncludeHeaders.size(), OS); for (const auto &Include : Sym.IncludeHeaders) @@ -361,7 +361,9 @@ return Sym; for (auto &I : Sym.IncludeHeaders) { I.IncludeHeader = Data.consumeString(Strings); - I.References = Data.consumeVar(); + uint32_t RefsWithDirectives = Data.consumeVar(); + I.References = RefsWithDirectives >> 2; + I.SupportedDirectives = RefsWithDirectives & 0x3; } return Sym; } @@ -455,7 +457,7 @@ // The current versioning scheme is simple - non-current versions are rejected. // If you make a breaking change, bump this version number to invalidate stored // data. Later we may want to support some backward compatibility. -constexpr static uint32_t Version = 17; +constexpr static uint32_t Version = 18; llvm::Expected readRIFF(llvm::StringRef Data, SymbolOrigin Origin) { diff --git a/clang-tools-extra/clangd/index/Symbol.h b/clang-tools-extra/clangd/index/Symbol.h --- a/clang-tools-extra/clangd/index/Symbol.h +++ b/clang-tools-extra/clangd/index/Symbol.h @@ -13,12 +13,15 @@ #include "index/SymbolLocation.h" #include "index/SymbolOrigin.h" #include "clang/Index/IndexSymbol.h" +#include "llvm/ADT/BitmaskEnum.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/StringSaver.h" namespace clang { namespace clangd { +LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); + /// The class presents a C++ symbol, e.g. class, function. /// /// WARNING: Symbols do not own much of their underlying data - typically @@ -84,12 +87,24 @@ /// Only set when the symbol is indexed for completion. llvm::StringRef Type; + enum IncludeDirective : uint8_t { + Invalid = 0, + /// `#include "header.h"` + Include = 1, + /// `#import "header.h"` + Import = 2, + + LLVM_MARK_AS_BITMASK_ENUM(Import) + }; + struct IncludeHeaderWithReferences { IncludeHeaderWithReferences() = default; IncludeHeaderWithReferences(llvm::StringRef IncludeHeader, - unsigned References) - : IncludeHeader(IncludeHeader), References(References) {} + uint32_t References, + IncludeDirective SupportedDirectives) + : IncludeHeader(IncludeHeader), References(References), + SupportedDirectives(SupportedDirectives) {} /// This can be either a URI of the header to be #include'd /// for this symbol, or a literal header quoted with <> or "" that is @@ -101,7 +116,14 @@ llvm::StringRef IncludeHeader = ""; /// The number of translation units that reference this symbol and include /// this header. This number is only meaningful if aggregated in an index. - unsigned References = 0; + uint32_t References : 30; + /// Bitfield of supported directives (IncludeDirective) that can be used + /// when including this header. + uint32_t SupportedDirectives : 2; + + IncludeDirective supportedDirectives() const { + return static_cast(SupportedDirectives); + } }; /// One Symbol can potentially be included via different headers. /// - If we haven't seen a definition, this covers all declarations. diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -153,6 +153,9 @@ // File IDs for Symbol.IncludeHeaders. // The final spelling is calculated in finish(). llvm::DenseMap IncludeFiles; + // Files which contain ObjC symbols. + // This is finalized and used in finish(). + llvm::DenseSet FilesWithObjCConstructs; void setIncludeLocation(const Symbol &S, SourceLocation); // Indexed macros, to be erased if they turned out to be include guards. llvm::DenseSet IndexedMacros; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -76,7 +76,7 @@ // We only collect #include paths for symbols that are suitable for global code // completion, except for namespaces since #include path for a namespace is hard // to define. -bool shouldCollectIncludePath(index::SymbolKind Kind) { +Symbol::IncludeDirective shouldCollectIncludePath(index::SymbolKind Kind) { using SK = index::SymbolKind; switch (Kind) { case SK::Macro: @@ -90,9 +90,11 @@ case SK::Variable: case SK::EnumConstant: case SK::Concept: - return true; + return Symbol::Include | Symbol::Import; + case SK::Protocol: + return Symbol::Import; default: - return false; + return Symbol::Invalid; } } @@ -805,12 +807,12 @@ } void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation Loc) { - if (Opts.CollectIncludePath) - if (shouldCollectIncludePath(S.SymInfo.Kind)) - // Use the expansion location to get the #include header since this is - // where the symbol is exposed. - IncludeFiles[S.ID] = - PP->getSourceManager().getDecomposedExpansionLoc(Loc).first; + if (Opts.CollectIncludePath && + shouldCollectIncludePath(S.SymInfo.Kind) != Symbol::Invalid) + // Use the expansion location to get the #include header since this is + // where the symbol is exposed. + IncludeFiles[S.ID] = + PP->getSourceManager().getDecomposedExpansionLoc(Loc).first; } void SymbolCollector::finish() { @@ -835,11 +837,12 @@ Symbols.erase(ID); } } + llvm::DenseMap FileToContainsImportsOrObjC; // Fill in IncludeHeaders. // We delay this until end of TU so header guards are all resolved. llvm::SmallString<128> QName; - for (const auto &Entry : IncludeFiles) { - if (const Symbol *S = Symbols.find(Entry.first)) { + for (const auto &[SID, FID] : IncludeFiles) { + if (const Symbol *S = Symbols.find(SID)) { llvm::StringRef IncludeHeader; // Look for an overridden include header for this symbol specifically. if (Opts.Includes) { @@ -856,19 +859,36 @@ } // Otherwise find the approprate include header for the defining file. if (IncludeHeader.empty()) - IncludeHeader = HeaderFileURIs->getIncludeHeader(Entry.second); + IncludeHeader = HeaderFileURIs->getIncludeHeader(FID); // Symbols in slabs aren't mutable, insert() has to walk all the strings if (!IncludeHeader.empty()) { - Symbol NewSym = *S; - NewSym.IncludeHeaders.push_back({IncludeHeader, 1}); - Symbols.insert(NewSym); + Symbol::IncludeDirective Directives = Symbol::Invalid; + auto CollectDirectives = shouldCollectIncludePath(S->SymInfo.Kind); + if ((CollectDirectives & Symbol::Include) != 0) + Directives |= Symbol::Include; + // Only allow #import for symbols from ObjC-like files. + if ((CollectDirectives & Symbol::Import) != 0) { + auto [It, Inserted] = FileToContainsImportsOrObjC.try_emplace(FID); + if (Inserted) + It->second = FilesWithObjCConstructs.contains(FID) || + tooling::codeContainsImports( + ASTCtx->getSourceManager().getBufferData(FID)); + if (It->second) + Directives |= Symbol::Import; + } + if (Directives != Symbol::Invalid) { + Symbol NewSym = *S; + NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives}); + Symbols.insert(NewSym); + } } } } ReferencedSymbols.clear(); IncludeFiles.clear(); + FilesWithObjCConstructs.clear(); } const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, @@ -896,7 +916,8 @@ auto Loc = nameLocation(ND, SM); assert(Loc.isValid() && "Invalid source location for NamedDecl"); // FIXME: use the result to filter out symbols. - shouldIndexFile(SM.getFileID(Loc)); + auto FID = SM.getFileID(Loc); + shouldIndexFile(FID); if (auto DeclLoc = getTokenLocation(Loc)) S.CanonicalDeclaration = *DeclLoc; @@ -940,6 +961,8 @@ Symbols.insert(S); setIncludeLocation(S, ND.getLocation()); + if (S.SymInfo.Lang == index::SymbolLanguage::ObjC) + FilesWithObjCConstructs.insert(FID); return Symbols.find(S.ID); } diff --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp b/clang-tools-extra/clangd/index/YAMLSerialization.cpp --- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp +++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp @@ -28,8 +28,13 @@ #include "llvm/Support/raw_ostream.h" #include +namespace { +struct YIncludeHeaderWithReferences; +} + LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Symbol::IncludeHeaderWithReferences) LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Ref) +LLVM_YAML_IS_SEQUENCE_VECTOR(YIncludeHeaderWithReferences) namespace { using RefBundle = @@ -48,6 +53,21 @@ uint32_t Line; uint32_t Column; }; +// A class helps YAML to serialize the IncludeHeaderWithReferences as YAMLIO +// can't directly map bitfields. +struct YIncludeHeaderWithReferences { + llvm::StringRef IncludeHeader; + uint32_t References; + clang::clangd::Symbol::IncludeDirective SupportedDirectives; + + YIncludeHeaderWithReferences() = default; + + YIncludeHeaderWithReferences( + llvm::StringRef IncludeHeader, uint32_t References, + clang::clangd::Symbol::IncludeDirective SupportedDirectives) + : IncludeHeader(IncludeHeader), References(References), + SupportedDirectives(SupportedDirectives) {} +}; // avoid ODR violation of specialization for non-owned CompileCommand struct CompileCommandYAML : clang::tooling::CompileCommand {}; @@ -165,13 +185,40 @@ } }; -template <> -struct MappingTraits { - static void mapping(IO &IO, - clang::clangd::Symbol::IncludeHeaderWithReferences &Inc) { +template <> struct ScalarBitSetTraits { + static void bitset(IO &IO, clang::clangd::Symbol::IncludeDirective &Value) { + IO.bitSetCase(Value, "Include", clang::clangd::Symbol::Include); + IO.bitSetCase(Value, "Import", clang::clangd::Symbol::Import); + } +}; + +template <> struct MappingTraits { + static void mapping(IO &IO, YIncludeHeaderWithReferences &Inc) { IO.mapRequired("Header", Inc.IncludeHeader); IO.mapRequired("References", Inc.References); + IO.mapOptional("Directives", Inc.SupportedDirectives, + clang::clangd::Symbol::Include); + } +}; + +struct NormalizedIncludeHeaders { + using IncludeHeader = clang::clangd::Symbol::IncludeHeaderWithReferences; + NormalizedIncludeHeaders(IO &) {} + NormalizedIncludeHeaders( + IO &, const llvm::SmallVector &IncludeHeaders) { + for (auto &I : IncludeHeaders) { + Headers.emplace_back(I.IncludeHeader, I.References, + I.supportedDirectives()); + } + } + + llvm::SmallVector denormalize(IO &) { + llvm::SmallVector Result; + for (auto &H : Headers) + Result.emplace_back(H.IncludeHeader, H.References, H.SupportedDirectives); + return Result; } + llvm::SmallVector Headers; }; template <> struct MappingTraits { @@ -179,6 +226,10 @@ MappingNormalization NSymbolID(IO, Sym.ID); MappingNormalization NSymbolFlag( IO, Sym.Flags); + MappingNormalization< + NormalizedIncludeHeaders, + llvm::SmallVector> + NIncludeHeaders(IO, Sym.IncludeHeaders); IO.mapRequired("ID", NSymbolID->HexString); IO.mapRequired("Name", Sym.Name); IO.mapRequired("Scope", Sym.Scope); @@ -195,7 +246,7 @@ IO.mapOptional("Documentation", Sym.Documentation); IO.mapOptional("ReturnType", Sym.ReturnType); IO.mapOptional("Type", Sym.Type); - IO.mapOptional("IncludeHeaders", Sym.IncludeHeaders); + IO.mapOptional("IncludeHeaders", NIncludeHeaders->Headers); } }; diff --git a/clang-tools-extra/clangd/index/remote/Index.proto b/clang-tools-extra/clangd/index/remote/Index.proto --- a/clang-tools-extra/clangd/index/remote/Index.proto +++ b/clang-tools-extra/clangd/index/remote/Index.proto @@ -107,6 +107,7 @@ message HeaderWithReferences { optional string header = 1; optional uint32 references = 2; + optional uint32 supported_directives = 3; } message RelationsRequest { diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp @@ -406,6 +406,7 @@ const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader) { HeaderWithReferences Result; Result.set_references(IncludeHeader.References); + Result.set_supported_directives(IncludeHeader.SupportedDirectives); const std::string Header = IncludeHeader.IncludeHeader.str(); if (isLiteralInclude(Header)) { Result.set_header(Header); @@ -427,8 +428,12 @@ return URIString.takeError(); Header = *URIString; } - return clangd::Symbol::IncludeHeaderWithReferences{Strings.save(Header), - Message.references()}; + auto Directives = clangd::Symbol::IncludeDirective::Include; + if (Message.has_supported_directives()) + Directives = static_cast( + Message.supported_directives()); + return clangd::Symbol::IncludeHeaderWithReferences{ + Strings.save(Header), Message.references(), Directives}; } } // namespace remote diff --git a/clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx b/clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 GIT binary patch literal 0 Hc$@newText == InsertedText; +} MATCHER(insertInclude, "") { return !arg.Includes.empty() && bool(arg.Includes[0].Insertion); } @@ -812,7 +816,7 @@ Symbol Sym = cls("ns::X"); Sym.CanonicalDeclaration.FileURI = BarURI.c_str(); - Sym.IncludeHeaders.emplace_back(BarURI, 1); + Sym.IncludeHeaders.emplace_back(BarURI, 1, Symbol::Include); // Shorten include path based on search directory and insert. Annotations Test("int main() { ns::^ }"); TU.Code = Test.code().str(); @@ -843,8 +847,8 @@ auto BarURI = URI::create(BarHeader).toString(); SymX.CanonicalDeclaration.FileURI = BarURI.c_str(); SymY.CanonicalDeclaration.FileURI = BarURI.c_str(); - SymX.IncludeHeaders.emplace_back("", 1); - SymY.IncludeHeaders.emplace_back("", 1); + SymX.IncludeHeaders.emplace_back("", 1, Symbol::Include); + SymY.IncludeHeaders.emplace_back("", 1, Symbol::Include); // Shorten include path based on search directory and insert. auto Results = completions(R"cpp( namespace ns { @@ -1867,7 +1871,7 @@ // Differences in header-to-insert suppress bundling. std::string DeclFile = URI::create(testPath("foo")).toString(); NoArgsGFunc.CanonicalDeclaration.FileURI = DeclFile.c_str(); - NoArgsGFunc.IncludeHeaders.emplace_back("", 1); + NoArgsGFunc.IncludeHeaders.emplace_back("", 1, Symbol::Include); EXPECT_THAT( completions(Context + "int y = GFunc^", {NoArgsGFunc}, Opts).Completions, UnorderedElementsAre(AllOf(named("GFuncC"), insertInclude("")), @@ -1901,8 +1905,8 @@ SymX.CanonicalDeclaration.FileURI = BarURI.c_str(); SymY.CanonicalDeclaration.FileURI = BarURI.c_str(); // The include header is different, but really it's the same file. - SymX.IncludeHeaders.emplace_back("\"bar.h\"", 1); - SymY.IncludeHeaders.emplace_back(BarURI.c_str(), 1); + SymX.IncludeHeaders.emplace_back("\"bar.h\"", 1, Symbol::Include); + SymY.IncludeHeaders.emplace_back(BarURI.c_str(), 1, Symbol::Include); auto Results = completions("void f() { ::ns::^ }", {SymX, SymY}, Opts); // Expect both results are bundled, despite the different-but-same @@ -2699,8 +2703,8 @@ std::string DeclFile = URI::create(testPath("foo")).toString(); Symbol Sym = func("Func"); Sym.CanonicalDeclaration.FileURI = DeclFile.c_str(); - Sym.IncludeHeaders.emplace_back("\"foo.h\"", 2); - Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000); + Sym.IncludeHeaders.emplace_back("\"foo.h\"", 2, Symbol::Include); + Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000, Symbol::Include); auto Results = completions("Fun^", {Sym}).Completions; assert(!Results.empty()); @@ -2708,6 +2712,30 @@ EXPECT_EQ(Results[0].Includes.size(), 2u); } +TEST(CompletionTest, InsertIncludeOrImport) { + std::string DeclFile = URI::create(testPath("foo")).toString(); + Symbol Sym = func("Func"); + Sym.CanonicalDeclaration.FileURI = DeclFile.c_str(); + Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000, + Symbol::Include | Symbol::Import); + + auto Results = completions("Fun^", {Sym}).Completions; + assert(!Results.empty()); + EXPECT_THAT(Results[0], + AllOf(named("Func"), insertIncludeText("#include \"bar.h\"\n"))); + + Results = completions("Fun^", {Sym}, {}, "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"))); + + Sym.IncludeHeaders[0].SupportedDirectives = Symbol::Import; + Results = completions("Fun^", {Sym}).Completions; + assert(!Results.empty()); + EXPECT_THAT(Results[0], AllOf(named("Func"), Not(insertInclude()))); +} + TEST(CompletionTest, NoInsertIncludeIfOnePresent) { Annotations Test(R"cpp( #include "foo.h" @@ -2719,8 +2747,8 @@ std::string DeclFile = URI::create(testPath("foo")).toString(); Symbol Sym = func("Func"); Sym.CanonicalDeclaration.FileURI = DeclFile.c_str(); - Sym.IncludeHeaders.emplace_back("\"foo.h\"", 2); - Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000); + Sym.IncludeHeaders.emplace_back("\"foo.h\"", 2, Symbol::Include); + Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000, Symbol::Include); EXPECT_THAT(completions(TU, Test.point(), {Sym}).Completions, UnorderedElementsAre(AllOf(named("Func"), hasInclude("\"foo.h\""), diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -1071,7 +1071,7 @@ Sym.Flags |= Symbol::IndexedForCodeCompletion; Sym.CanonicalDeclaration.FileURI = S.DeclaringFile.c_str(); Sym.Definition.FileURI = S.DeclaringFile.c_str(); - Sym.IncludeHeaders.emplace_back(S.IncludeHeader, 1); + Sym.IncludeHeaders.emplace_back(S.IncludeHeader, 1, Symbol::Include); Slab.insert(Sym); } return MemIndex::build(std::move(Slab).build(), RefSlab(), RelationSlab()); @@ -1129,7 +1129,7 @@ Symbol Sym = enm("X"); Sym.Flags |= Symbol::IndexedForCodeCompletion; Sym.CanonicalDeclaration.FileURI = Sym.Definition.FileURI = "unittest:///x.h"; - Sym.IncludeHeaders.emplace_back("\"x.h\"", 1); + Sym.IncludeHeaders.emplace_back("\"x.h\"", 1, Symbol::Include); SymbolSlab::Builder Slab; Slab.insert(Sym); auto Index = @@ -1172,7 +1172,7 @@ Sym.Flags |= Symbol::IndexedForCodeCompletion; Sym.CanonicalDeclaration.FileURI = "unittest:///x.h"; Sym.Definition.FileURI = "unittest:///x.cc"; - Sym.IncludeHeaders.emplace_back("\"x.h\"", 1); + Sym.IncludeHeaders.emplace_back("\"x.h\"", 1, Symbol::Include); SymbolSlab::Builder Slab; Slab.insert(Sym); @@ -1503,7 +1503,7 @@ Symbol Sym = func("foo"); Sym.Flags |= Symbol::IndexedForCodeCompletion; Sym.CanonicalDeclaration.FileURI = "unittest:///foo.h"; - Sym.IncludeHeaders.emplace_back("\"foo.h\"", 1); + Sym.IncludeHeaders.emplace_back("\"foo.h\"", 1, Symbol::Include); SymbolSlab::Builder Slab; Slab.insert(Sym); diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp b/clang-tools-extra/clangd/unittests/IndexTests.cpp --- a/clang-tools-extra/clangd/unittests/IndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp @@ -567,9 +567,9 @@ L.Name = "left"; R.Name = "right"; L.ID = R.ID = SymbolID("hello"); - L.IncludeHeaders.emplace_back("common", 1); - R.IncludeHeaders.emplace_back("common", 1); - R.IncludeHeaders.emplace_back("new", 1); + L.IncludeHeaders.emplace_back("common", 1, Symbol::Include); + R.IncludeHeaders.emplace_back("common", 1, Symbol::Include); + R.IncludeHeaders.emplace_back("new", 1, Symbol::Include); // Both have no definition. Symbol M = mergeSymbol(L, R); @@ -615,7 +615,7 @@ std::move(DynData), DynSize); SymbolSlab::Builder StaticB; - S.IncludeHeaders.push_back({"
", 0}); + S.IncludeHeaders.push_back({"
", 0, Symbol::Include}); StaticB.insert(S); auto StaticIndex = MemIndex::build(std::move(StaticB).build(), RefSlab(), RelationSlab()); diff --git a/clang-tools-extra/clangd/unittests/SerializationTests.cpp b/clang-tools-extra/clangd/unittests/SerializationTests.cpp --- a/clang-tools-extra/clangd/unittests/SerializationTests.cpp +++ b/clang-tools-extra/clangd/unittests/SerializationTests.cpp @@ -53,8 +53,16 @@ IncludeHeaders: - Header: 'include1' References: 7 + Directives: [ Include ] - Header: 'include2' References: 3 + Directives: [ Import ] + - Header: 'include3' + References: 2 + Directives: [ Include, Import ] + - Header: 'include4' + References: 1 + Directives: [ ] ... --- !Symbol @@ -114,8 +122,11 @@ MATCHER_P(id, I, "") { return arg.ID == cantFail(SymbolID::fromStr(I)); } MATCHER_P(qName, Name, "") { return (arg.Scope + arg.Name).str() == Name; } -MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") { - return (arg.IncludeHeader == IncludeHeader) && (arg.References == References); +MATCHER_P3(IncludeHeaderWithRefAndDirectives, IncludeHeader, References, + SupportedDirectives, "") { + return (arg.IncludeHeader == IncludeHeader) && + (arg.References == References) && + (arg.SupportedDirectives == SupportedDirectives); } auto readIndexFile(llvm::StringRef Text) { @@ -148,9 +159,14 @@ EXPECT_EQ(static_cast(Sym1.Flags), 129); EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion); EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated); - EXPECT_THAT(Sym1.IncludeHeaders, - UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u), - IncludeHeaderWithRef("include2", 3u))); + EXPECT_THAT( + Sym1.IncludeHeaders, + UnorderedElementsAre( + IncludeHeaderWithRefAndDirectives("include1", 7u, Symbol::Include), + IncludeHeaderWithRefAndDirectives("include2", 3u, Symbol::Import), + IncludeHeaderWithRefAndDirectives("include3", 2u, + Symbol::Include | Symbol::Import), + IncludeHeaderWithRefAndDirectives("include4", 1u, Symbol::Invalid))); EXPECT_THAT(Sym2, qName("clang::Foo2")); EXPECT_EQ(Sym2.Signature, "-sig"); diff --git a/clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h b/clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h --- a/clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h +++ b/clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h @@ -21,7 +21,7 @@ /// Returns true if the given physical file is a self-contained header. /// /// A header is considered self-contained if -// - it has a proper header guard or has been #imported +// - it has a proper header guard or has been #imported or contains #import(s) // - *and* it doesn't have a dont-include-me pattern. /// /// This function can be expensive as it may scan the source code to find out @@ -29,6 +29,9 @@ bool isSelfContainedHeader(const FileEntry *FE, const SourceManager &SM, HeaderSearch &HeaderInfo); +/// This scans the given source code to see if it contains #import(s). +bool codeContainsImports(llvm::StringRef Code); + /// If Text begins an Include-What-You-Use directive, returns it. /// Given "// IWYU pragma: keep", returns "keep". /// Input is a null-terminated char* as provided by SM.getCharacterData(). diff --git a/clang/lib/Tooling/Inclusions/HeaderAnalysis.cpp b/clang/lib/Tooling/Inclusions/HeaderAnalysis.cpp --- a/clang/lib/Tooling/Inclusions/HeaderAnalysis.cpp +++ b/clang/lib/Tooling/Inclusions/HeaderAnalysis.cpp @@ -37,8 +37,7 @@ } // Heuristically headers that only want to be included via an umbrella. -bool isDontIncludeMeHeader(llvm::MemoryBufferRef Buffer) { - StringRef Content = Buffer.getBuffer(); +bool isDontIncludeMeHeader(StringRef Content) { llvm::StringRef Line; // Only sniff up to 100 lines or 10KB. Content = Content.take_front(100 * 100); @@ -50,19 +49,48 @@ return false; } +bool isImportLine(llvm::StringRef Line) { + Line = Line.ltrim(); + if (!Line.consume_front("#")) + return false; + Line = Line.ltrim(); + return Line.startswith("import"); +} + +llvm::StringRef getFileContents(const FileEntry *FE, const SourceManager &SM) { + return const_cast(SM) + .getMemoryBufferForFileOrNone(FE) + .value_or(llvm::MemoryBufferRef()) + .getBuffer(); +} + } // namespace bool isSelfContainedHeader(const FileEntry *FE, const SourceManager &SM, HeaderSearch &HeaderInfo) { assert(FE); if (!HeaderInfo.isFileMultipleIncludeGuarded(FE) && - !HeaderInfo.hasFileBeenImported(FE)) + !HeaderInfo.hasFileBeenImported(FE) && + // Any header that contains #imports is supposed to be #import'd so no + // need to check for anything but the main-file. + (SM.getFileEntryForID(SM.getMainFileID()) != FE || + !codeContainsImports(getFileContents(FE, SM)))) return false; // This pattern indicates that a header can't be used without // particular preprocessor state, usually set up by another header. - return !isDontIncludeMeHeader( - const_cast(SM).getMemoryBufferForFileOrNone(FE).value_or( - llvm::MemoryBufferRef())); + return !isDontIncludeMeHeader(getFileContents(FE, SM)); +} + +bool codeContainsImports(llvm::StringRef Code) { + // Only sniff up to 100 lines or 10KB. + Code = Code.take_front(100 * 100); + llvm::StringRef Line; + for (unsigned I = 0; I < 100 && !Code.empty(); ++I) { + std::tie(Line, Code) = Code.split('\n'); + if (isImportLine(Line)) + return true; + } + return false; } llvm::Optional parseIWYUPragma(const char *Text) { diff --git a/clang/unittests/Tooling/HeaderAnalysisTest.cpp b/clang/unittests/Tooling/HeaderAnalysisTest.cpp --- a/clang/unittests/Tooling/HeaderAnalysisTest.cpp +++ b/clang/unittests/Tooling/HeaderAnalysisTest.cpp @@ -56,11 +56,42 @@ EXPECT_TRUE(isSelfContainedHeader(FM.getFile("headerguard.h").get(), SM, HI)); EXPECT_TRUE(isSelfContainedHeader(FM.getFile("pragmaonce.h").get(), SM, HI)); EXPECT_TRUE(isSelfContainedHeader(FM.getFile("imported.h").get(), SM, HI)); + EXPECT_TRUE( + isSelfContainedHeader(SM.getFileEntryForID(SM.getMainFileID()), SM, HI)); EXPECT_FALSE(isSelfContainedHeader(FM.getFile("unguarded.h").get(), SM, HI)); EXPECT_FALSE(isSelfContainedHeader(FM.getFile("bad.h").get(), SM, HI)); } +TEST(HeaderAnalysisTest, CodeContainsImports) { + EXPECT_TRUE(codeContainsImports(R"cpp( + #include "foo.h" + #import "NSFoo.h" + + int main() { + foo(); + } + )cpp")); + + EXPECT_TRUE(codeContainsImports(R"cpp( + #include "foo.h" + + int main() { + foo(); + } + + #import "NSFoo.h" + )cpp")); + + EXPECT_FALSE(codeContainsImports(R"cpp( + #include "foo.h" + + int main() { + foo(); + } + )cpp")); +} + TEST(HeaderAnalysisTest, ParseIWYUPragma) { EXPECT_THAT(parseIWYUPragma("// IWYU pragma: keep"), ValueIs(Eq("keep"))); EXPECT_THAT(parseIWYUPragma("// IWYU pragma: keep me\netc"),