diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 1471301a343179..079d5747721660 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -744,7 +744,7 @@ struct EqualClangTidyError { std::vector ClangTidyDiagnosticConsumer::take() { finalizeLastError(); - llvm::sort(Errors, LessClangTidyError()); + llvm::stable_sort(Errors, LessClangTidyError()); Errors.erase(std::unique(Errors.begin(), Errors.end(), EqualClangTidyError()), Errors.end()); if (RemoveIncompatibleErrors) diff --git a/clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h b/clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h index 3f7529d1bbd011..f58ff5bc44b214 100644 --- a/clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h +++ b/clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h @@ -50,8 +50,8 @@ AST_POLYMORPHIC_MATCHER( static const char *AbseilLibraries[] = { "algorithm", "base", "container", "debugging", "flags", "hash", "iterator", "memory", "meta", "numeric", - "random", "strings", "synchronization", "time", "types", - "utility"}; + "random", "strings", "synchronization", "status", "time", + "types", "utility"}; return std::any_of( std::begin(AbseilLibraries), std::end(AbseilLibraries), [&](const char *Library) { return Path.startswith(Library); }); diff --git a/clang-tools-extra/clang-tidy/add_new_check.py b/clang-tools-extra/clang-tidy/add_new_check.py index 231f43c0b8f3d1..14fcfe8d49ff33 100755 --- a/clang-tools-extra/clang-tidy/add_new_check.py +++ b/clang-tools-extra/clang-tidy/add_new_check.py @@ -136,7 +136,7 @@ def write_implementation(module_path, module, namespace, check_name_camel): void %(check_name)s::check(const MatchFinder::MatchResult &Result) { // FIXME: Add callback implementation. const auto *MatchedDecl = Result.Nodes.getNodeAs("x"); - if (MatchedDecl->getName().startswith("awesome_")) + if (!MatchedDecl->getIdentifier() || MatchedDecl->getName().startswith("awesome_")) return; diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome") << MatchedDecl; diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp index 7d484d777165fb..9a44ba2f58a5d0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp @@ -805,10 +805,12 @@ void NotNullTerminatedResultCheck::check( // PP->getMacroInfo() returns nullptr if macro has no definition. if (MI) { const auto &T = MI->tokens().back(); - StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength()); - llvm::APInt IntValue; - ValueStr.getAsInteger(10, IntValue); - AreSafeFunctionsWanted = IntValue.getZExtValue(); + if (T.isLiteral() && T.getLiteralData()) { + StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength()); + llvm::APInt IntValue; + ValueStr.getAsInteger(10, IntValue); + AreSafeFunctionsWanted = IntValue.getZExtValue(); + } } } diff --git a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp index 8e31b4b5f75fcf..b66e24d58b2f64 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp @@ -261,8 +261,8 @@ static bool hasAnyNestedLocalQualifiers(QualType Type) { } SourceRange UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange( - const FunctionDecl &F, const ASTContext &Ctx, const SourceManager &SM, - const LangOptions &LangOpts) { + const FunctionDecl &F, const TypeLoc &ReturnLoc, const ASTContext &Ctx, + const SourceManager &SM, const LangOptions &LangOpts) { // We start with the range of the return type and expand to neighboring // qualifiers (const, volatile and restrict). @@ -274,6 +274,35 @@ SourceRange UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange( return {}; } + // If the return type is a constrained 'auto' or 'decltype(auto)', we need to + // include the tokens after the concept. Unfortunately, the source range of an + // AutoTypeLoc, if it is constrained, does not include the 'auto' or + // 'decltype(auto)'. If the return type is a plain 'decltype(...)', the + // source range only contains the first 'decltype' token. + auto ATL = ReturnLoc.getAs(); + if ((ATL && (ATL.isConstrained() || + ATL.getAutoKeyword() == AutoTypeKeyword::DecltypeAuto)) || + ReturnLoc.getAs()) { + SourceLocation End = + expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM); + SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM); + + // Extend the ReturnTypeRange until the last token before the function + // name. + std::pair Loc = SM.getDecomposedLoc(End); + StringRef File = SM.getBufferData(Loc.first); + const char *TokenBegin = File.data() + Loc.second; + Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(), + TokenBegin, File.end()); + Token T; + SourceLocation LastTLoc = End; + while (!Lexer.LexFromRawLexer(T) && + SM.isBeforeInTranslationUnit(T.getLocation(), BeginNameF)) { + LastTLoc = T.getLocation(); + } + ReturnTypeRange.setEnd(LastTLoc); + } + // If the return type has no local qualifiers, it's source range is accurate. if (!hasAnyNestedLocalQualifiers(F.getReturnType())) return ReturnTypeRange; @@ -317,7 +346,7 @@ SourceRange UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange( return ReturnTypeRange; } -bool UseTrailingReturnTypeCheck::keepSpecifiers( +void UseTrailingReturnTypeCheck::keepSpecifiers( std::string &ReturnType, std::string &Auto, SourceRange ReturnTypeCVRange, const FunctionDecl &F, const FriendDecl *Fr, const ASTContext &Ctx, const SourceManager &SM, const LangOptions &LangOpts) { @@ -327,14 +356,14 @@ bool UseTrailingReturnTypeCheck::keepSpecifiers( if (!F.isConstexpr() && !F.isInlineSpecified() && F.getStorageClass() != SC_Extern && F.getStorageClass() != SC_Static && !Fr && !(M && M->isVirtualAsWritten())) - return true; + return; // Tokenize return type. If it contains macros which contain a mix of // qualifiers, specifiers and types, give up. llvm::Optional> MaybeTokens = classifyTokensBeforeFunctionName(F, Ctx, SM, LangOpts); if (!MaybeTokens) - return false; + return; // Find specifiers, remove them from the return type, add them to 'auto'. unsigned int ReturnTypeBeginOffset = @@ -367,14 +396,12 @@ bool UseTrailingReturnTypeCheck::keepSpecifiers( ReturnType.erase(TOffsetInRT, TLengthWithWS); DeletedChars += TLengthWithWS; } - - return true; } void UseTrailingReturnTypeCheck::registerMatchers(MatchFinder *Finder) { - auto F = functionDecl(unless(anyOf(hasTrailingReturn(), returns(voidType()), - returns(autoType()), cxxConversionDecl(), - cxxMethodDecl(isImplicit())))) + auto F = functionDecl( + unless(anyOf(hasTrailingReturn(), returns(voidType()), + cxxConversionDecl(), cxxMethodDecl(isImplicit())))) .bind("Func"); Finder->addMatcher(F, this); @@ -397,11 +424,17 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) { if (F->getLocation().isInvalid()) return; + // Skip functions which return just 'auto'. + const auto *AT = F->getDeclaredReturnType()->getAs(); + if (AT != nullptr && !AT->isConstrained() && + AT->getKeyword() == AutoTypeKeyword::Auto && + !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType())) + return; + // TODO: implement those if (F->getDeclaredReturnType()->isFunctionPointerType() || F->getDeclaredReturnType()->isMemberFunctionPointerType() || - F->getDeclaredReturnType()->isMemberPointerType() || - F->getDeclaredReturnType()->getAs() != nullptr) { + F->getDeclaredReturnType()->isMemberPointerType()) { diag(F->getLocation(), Message); return; } @@ -435,7 +468,7 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) { // discards user formatting and order of const, volatile, type, whitespace, // space before & ... . SourceRange ReturnTypeCVRange = - findReturnTypeAndCVSourceRange(*F, Ctx, SM, LangOpts); + findReturnTypeAndCVSourceRange(*F, FTL.getReturnLoc(), Ctx, SM, LangOpts); if (ReturnTypeCVRange.isInvalid()) return; diff --git a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h index 0cd4e7741c3e55..72643ee02ef4ee 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h @@ -50,10 +50,11 @@ class UseTrailingReturnTypeCheck : public ClangTidyCheck { const SourceManager &SM, const LangOptions &LangOpts); SourceRange findReturnTypeAndCVSourceRange(const FunctionDecl &F, + const TypeLoc &ReturnLoc, const ASTContext &Ctx, const SourceManager &SM, const LangOptions &LangOpts); - bool keepSpecifiers(std::string &ReturnType, std::string &Auto, + void keepSpecifiers(std::string &ReturnType, std::string &Auto, SourceRange ReturnTypeCVRange, const FunctionDecl &F, const FriendDecl *Fr, const ASTContext &Ctx, const SourceManager &SM, const LangOptions &LangOpts); diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp index 2c116b210d0501..6ea757a38f0548 100644 --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp @@ -53,12 +53,7 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R, void TransformerClangTidyCheck::registerPPCallbacks( const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - // Only register the IncludeInsert when some `Case` will add - // includes. - if (Rule && llvm::any_of(Rule->Cases, [](const RewriteRule::Case &C) { - return !C.AddedIncludes.empty(); - })) - Inserter.registerPreprocessor(PP); + Inserter.registerPreprocessor(PP); } void TransformerClangTidyCheck::registerMatchers( @@ -96,13 +91,19 @@ void TransformerClangTidyCheck::check( // Associate the diagnostic with the location of the first change. DiagnosticBuilder Diag = diag((*Edits)[0].Range.getBegin(), *Explanation); for (const auto &T : *Edits) - Diag << FixItHint::CreateReplacement(T.Range, T.Replacement); - - for (const auto &I : Case.AddedIncludes) { - Diag << Inserter.createMainFileIncludeInsertion( - I.first, - /*IsAngled=*/I.second == transformer::IncludeFormat::Angled); - } + switch (T.Kind) { + case transformer::EditKind::Range: + Diag << FixItHint::CreateReplacement(T.Range, T.Replacement); + break; + case transformer::EditKind::AddInclude: { + StringRef FileName = T.Replacement; + bool IsAngled = FileName.startswith("<") && FileName.endswith(">"); + Diag << Inserter.createMainFileIncludeInsertion( + IsAngled ? FileName.substr(1, FileName.size() - 2) : FileName, + IsAngled); + break; + } + } } void TransformerClangTidyCheck::storeOptions( diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index ec48556595010e..74ab21a5f7788f 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -40,6 +40,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Errc.h" #include "llvm/Support/Error.h" @@ -52,6 +53,7 @@ #include #include #include +#include #include namespace clang { @@ -109,6 +111,41 @@ struct UpdateIndexCallbacks : public ParsingCallbacks { ClangdServer::Callbacks *ServerCallbacks; bool TheiaSemanticHighlighting; }; + +// Set of clang-tidy checks that don't work well in clangd, either due to +// crashes or false positives. +// Returns a clang-tidy filter string: -check1,-check2. +llvm::StringRef getUnusableTidyChecks() { + static const std::string FalsePositives = + llvm::join_items(", ", + // Check relies on seeing ifndef/define/endif directives, + // clangd doesn't replay those when using a preamble. + "-llvm-header-guard"); + static const std::string CrashingChecks = + llvm::join_items(", ", + // Check can choke on invalid (intermediate) c++ code, + // which is often the case when clangd tries to build an + // AST. + "-bugprone-use-after-move"); + static const std::string UnusableTidyChecks = + llvm::join_items(", ", FalsePositives, CrashingChecks); + return UnusableTidyChecks; +} + +// Returns a clang-tidy options string: check1,check2. +llvm::StringRef getDefaultTidyChecks() { + // These default checks are chosen for: + // - low false-positive rate + // - providing a lot of value + // - being reasonably efficient + static const std::string DefaultChecks = llvm::join_items( + ",", "readability-misleading-indentation", "readability-deleted-default", + "bugprone-integer-division", "bugprone-sizeof-expression", + "bugprone-suspicious-missing-comma", "bugprone-unused-raii", + "bugprone-unused-return-value", "misc-unused-using-decls", + "misc-unused-alias-decls", "misc-definitions-in-headers"); + return DefaultChecks; +} } // namespace ClangdServer::Options ClangdServer::optsForTest() { @@ -171,16 +208,20 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, if (Opts.StaticIndex) AddIndex(Opts.StaticIndex); if (Opts.BackgroundIndex) { + BackgroundIndex::Options BGOpts; + BGOpts.ThreadPoolSize = std::max(Opts.AsyncThreadsCount, 1u); + BGOpts.OnProgress = [Callbacks](BackgroundQueue::Stats S) { + if (Callbacks) + Callbacks->onBackgroundIndexProgress(S); + }; + BGOpts.ContextProvider = [this](PathRef P) { + return createProcessingContext(P); + }; BackgroundIdx = std::make_unique( - Context::current().clone(), TFS, CDB, + TFS, CDB, BackgroundIndexStorage::createDiskBackedStorageFactory( [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }), - std::max(Opts.AsyncThreadsCount, 1u), - [Callbacks](BackgroundQueue::Stats S) { - if (Callbacks) - Callbacks->onBackgroundIndexProgress(S); - }, - [this](PathRef P) { return createProcessingContext(P); }); + std::move(BGOpts)); AddIndex(BackgroundIdx.get()); } if (DynamicIdx) @@ -196,6 +237,15 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents, if (GetClangTidyOptions) Opts.ClangTidyOpts = GetClangTidyOptions(*TFS.view(/*CWD=*/llvm::None), File); + if (Opts.ClangTidyOpts.Checks.hasValue()) { + // If the set of checks was configured, make sure clangd incompatible ones + // are disabled. + Opts.ClangTidyOpts.Checks = llvm::join_items( + ", ", *Opts.ClangTidyOpts.Checks, getUnusableTidyChecks()); + } else { + // Otherwise provide a nice set of defaults. + Opts.ClangTidyOpts.Checks = getDefaultTidyChecks().str(); + } Opts.SuggestMissingIncludes = SuggestMissingIncludes; // Compile command is set asynchronously during update, as it can be slow. @@ -525,26 +575,6 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID, WorkScheduler.runWithAST("ApplyTweak", File, std::move(Action)); } -void ClangdServer::dumpAST(PathRef File, - llvm::unique_function Callback) { - auto Action = [Callback = std::move(Callback)]( - llvm::Expected InpAST) mutable { - if (!InpAST) { - llvm::consumeError(InpAST.takeError()); - return Callback(""); - } - std::string Result; - - llvm::raw_string_ostream ResultOS(Result); - clangd::dumpAST(InpAST->AST, ResultOS); - ResultOS.flush(); - - Callback(Result); - }; - - WorkScheduler.runWithAST("DumpAST", File, std::move(Action)); -} - void ClangdServer::locateSymbolAt(PathRef File, Position Pos, Callback> CB) { auto Action = [Pos, CB = std::move(CB), @@ -754,6 +784,11 @@ void ClangdServer::semanticHighlights( TUScheduler::InvalidateOnUpdate); } +void ClangdServer::customAction(PathRef File, llvm::StringRef Name, + Callback Action) { + WorkScheduler.runWithAST(Name, File, std::move(Action)); +} + llvm::StringMap ClangdServer::fileStats() const { return WorkScheduler.fileStats(); } diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 3529e5050aa380..1bc7d70eebaddc 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -295,10 +295,6 @@ class ClangdServer { void applyTweak(PathRef File, Range Sel, StringRef ID, Callback CB); - /// Only for testing purposes. - /// Waits until all requests to worker thread are finished and dumps AST for - /// \p File. \p File must be in the list of added documents. - void dumpAST(PathRef File, llvm::unique_function Callback); /// Called when an event occurs for a watched file in the workspace. void onFileEvent(const DidChangeWatchedFilesParams &Params); @@ -317,6 +313,13 @@ class ClangdServer { void semanticHighlights(PathRef File, Callback>); + /// Runs an arbitrary action that has access to the AST of the specified file. + /// The action will execute on one of ClangdServer's internal threads. + /// The AST is only valid for the duration of the callback. + /// As with other actions, the file must have been opened. + void customAction(PathRef File, llvm::StringRef Name, + Callback Action); + /// Returns estimated memory usage and other statistics for each of the /// currently open files. /// Overall memory usage of clangd may be significantly more than reported diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index f79033ac951420..92ebc4c39f64cb 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1617,8 +1617,10 @@ class CodeCompleteFlow { llvm::Optional fuzzyScore(const CompletionCandidate &C) { // Macros can be very spammy, so we only support prefix completion. - // We won't end up with underfull index results, as macros are sema-only. - if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro && + if (((C.SemaResult && + C.SemaResult->Kind == CodeCompletionResult::RK_Macro) || + (C.IndexResult && + C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) && !C.Name.startswith_lower(Filter->pattern())) return None; return Filter->match(C.Name); diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp index e4d2dddb4b5d3f..f73a6e58497274 100644 --- a/clang-tools-extra/clangd/FindTarget.cpp +++ b/clang-tools-extra/clangd/FindTarget.cpp @@ -355,7 +355,21 @@ struct TargetFinder { Flags |= Rel::Underlying; // continue with the underlying decl. } else if (const auto *DG = dyn_cast(D)) { D = DG->getDeducedTemplate(); + } else if (const ObjCImplementationDecl *IID = + dyn_cast(D)) { + // Treat ObjC{Interface,Implementation}Decl as if they were a decl/def + // pair as long as the interface isn't implicit. + if (const auto *CID = IID->getClassInterface()) + if (const auto *DD = CID->getDefinition()) + if (!DD->isImplicitInterfaceDecl()) + D = DD; + } else if (const ObjCCategoryImplDecl *CID = + dyn_cast(D)) { + // Treat ObjC{Category,CategoryImpl}Decl as if they were a decl/def pair. + D = CID->getCategoryDecl(); } + if (!D) + return; if (const Decl *Pat = getTemplatePattern(D)) { assert(Pat != D); @@ -596,6 +610,19 @@ struct TargetFinder { add(CCI->getAnyMember(), Flags); // Constructor calls contain a TypeLoc node, so we don't handle them here. } + + void add(const TemplateArgument &Arg, RelSet Flags) { + // Only used for template template arguments. + // For type and non-type template arguments, SelectionTree + // will hit a more specific node (e.g. a TypeLoc or a + // DeclRefExpr). + if (Arg.getKind() == TemplateArgument::Template || + Arg.getKind() == TemplateArgument::TemplateExpansion) { + if (TemplateDecl *TD = Arg.getAsTemplate().getAsTemplateDecl()) { + report(TD, Flags); + } + } + } }; } // namespace @@ -619,6 +646,8 @@ allTargetDecls(const ast_type_traits::DynTypedNode &N) { Finder.add(*QT, Flags); else if (const CXXCtorInitializer *CCI = N.get()) Finder.add(CCI, Flags); + else if (const TemplateArgumentLoc *TAL = N.get()) + Finder.add(TAL->getArgument(), Flags); return Finder.takeDecls(); } diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 7a7853a6245c8b..23d86221e74fee 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -238,10 +238,6 @@ class ReplayPreamble : private PPCallbacks { } // namespace -void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) { - AST.getASTContext().getTranslationUnitDecl()->dump(OS, true); -} - llvm::Optional ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, std::unique_ptr CI, @@ -428,15 +424,15 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, CTFinder.matchAST(Clang->getASTContext()); } + // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF. + // However Action->EndSourceFile() would destroy the ASTContext! + // So just inform the preprocessor of EOF, while keeping everything alive. + Clang->getPreprocessor().EndSourceFile(); // UnitDiagsConsumer is local, we can not store it in CompilerInstance that // has a longer lifetime. Clang->getDiagnostics().setClient(new IgnoreDiagnostics); // CompilerInstance won't run this callback, do it directly. ASTDiags.EndSourceFile(); - // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF. - // However Action->EndSourceFile() would destroy the ASTContext! - // So just inform the preprocessor of EOF, while keeping everything alive. - Clang->getPreprocessor().EndSourceFile(); std::vector Diags = CompilerInvocationDiags; // Add diagnostics from the preamble, if any. diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index 361b20aeff4cb9..05818b9dba80ec 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -146,10 +146,6 @@ class ParsedAST { CanonicalIncludes CanonIncludes; }; -/// For testing/debugging purposes. Note that this method deserializes all -/// unserialized Decls, so use with care. -void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS); - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/RIFF.cpp b/clang-tools-extra/clangd/RIFF.cpp index b87c2d56af0c5b..f59200bd585611 100644 --- a/clang-tools-extra/clangd/RIFF.cpp +++ b/clang-tools-extra/clangd/RIFF.cpp @@ -33,7 +33,7 @@ llvm::Expected readChunk(llvm::StringRef &Stream) { llvm::Twine(Stream.size())); C.Data = Stream.take_front(Len); Stream = Stream.drop_front(Len); - if (Len % 2 & !Stream.empty()) { // Skip padding byte. + if ((Len % 2) && !Stream.empty()) { // Skip padding byte. if (Stream.front()) return makeError("nonzero padding byte"); Stream = Stream.drop_front(); diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp index 2e3c491d561e70..5c08485d8b9200 100644 --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -479,6 +479,10 @@ class SelectionVisitor : public RecursiveASTVisitor { bool TraverseTypeLoc(TypeLoc X) { return traverseNode(&X, [&] { return Base::TraverseTypeLoc(X); }); } + bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &X) { + return traverseNode(&X, + [&] { return Base::TraverseTemplateArgumentLoc(X); }); + } bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc X) { return traverseNode( &X, [&] { return Base::TraverseNestedNameSpecifierLoc(X); }); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index b61ff4979320a9..9936c67cb6e5b2 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -78,6 +78,32 @@ const NamedDecl *getDefinition(const NamedDecl *D) { return VD->getDefinition(); if (const auto *FD = dyn_cast(D)) return FD->getDefinition(); + // Objective-C classes can have three types of declarations: + // + // - forward declaration: @class MyClass; + // - true declaration (interface definition): @interface MyClass ... @end + // - true definition (implementation): @implementation MyClass ... @end + // + // Objective-C categories are extensions are on classes: + // + // - declaration: @interface MyClass (Ext) ... @end + // - definition: @implementation MyClass (Ext) ... @end + // + // With one special case, a class extension, which is normally used to keep + // some declarations internal to a file without exposing them in a header. + // + // - class extension declaration: @interface MyClass () ... @end + // - which really links to class definition: @implementation MyClass ... @end + if (const auto *ID = dyn_cast(D)) + return ID->getImplementation(); + if (const auto *CD = dyn_cast(D)) { + if (CD->IsClassExtension()) { + if (const auto *ID = CD->getClassInterface()) + return ID->getImplementation(); + return nullptr; + } + return CD->getImplementation(); + } // Only a single declaration is allowed. if (isa(D) || isa(D) || isa(D)) // except cases above @@ -223,6 +249,37 @@ locateMacroReferent(const syntax::Token &TouchedIdentifier, ParsedAST &AST, return llvm::None; } +// A wrapper around `Decl::getCanonicalDecl` to support cases where Clang's +// definition of a canonical declaration doesn't match up to what a programmer +// would expect. For example, Objective-C classes can have three types of +// declarations: +// +// - forward declaration(s): @class MyClass; +// - true declaration (interface definition): @interface MyClass ... @end +// - true definition (implementation): @implementation MyClass ... @end +// +// Clang will consider the forward declaration to be the canonical declaration +// because it is first. We actually want the class definition if it is +// available since that is what a programmer would consider the primary +// declaration to be. +const NamedDecl *getPreferredDecl(const NamedDecl *D) { + // FIXME: Canonical declarations of some symbols might refer to built-in + // decls with possibly-invalid source locations (e.g. global new operator). + // In such cases we should pick up a redecl with valid source location + // instead of failing. + D = llvm::cast(D->getCanonicalDecl()); + + // Prefer Objective-C class/protocol definitions over the forward declaration. + if (const auto *ID = dyn_cast(D)) + if (const auto *DefinitionID = ID->getDefinition()) + return DefinitionID; + if (const auto *PD = dyn_cast(D)) + if (const auto *DefinitionID = PD->getDefinition()) + return DefinitionID; + + return D; +} + // Decls are more complicated. // The AST contains at least a declaration, maybe a definition. // These are up-to-date, and so generally preferred over index results. @@ -238,11 +295,7 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier, llvm::DenseMap ResultIndex; auto AddResultDecl = [&](const NamedDecl *D) { - // FIXME: Canonical declarations of some symbols might refer to built-in - // decls with possibly-invalid source locations (e.g. global new operator). - // In such cases we should pick up a redecl with valid source location - // instead of failing. - D = llvm::cast(D->getCanonicalDecl()); + D = getPreferredDecl(D); auto Loc = makeLocation(AST.getASTContext(), nameLocation(*D, SM), MainFilePath); if (!Loc) @@ -302,6 +355,19 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier, continue; } + // Special case: if the class name is selected, also map Objective-C + // categories and category implementations back to their class interface. + // + // Since `TouchedIdentifier` might refer to the `ObjCCategoryImplDecl` + // instead of the `ObjCCategoryDecl` we intentionally check the contents + // of the locs when checking for class name equivalence. + if (const auto *CD = dyn_cast(D)) + if (const auto *ID = CD->getClassInterface()) + if (TouchedIdentifier && + (CD->getLocation() == TouchedIdentifier->location() || + ID->getName() == TouchedIdentifier->text(SM))) + AddResultDecl(ID); + // Otherwise the target declaration is the right one. AddResultDecl(D); } diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp index a22785b01d6475..18037d694c11ed 100644 --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -91,28 +91,25 @@ bool shardIsStale(const LoadedShard &LS, llvm::vfs::FileSystem *FS) { } // namespace BackgroundIndex::BackgroundIndex( - Context BackgroundContext, const ThreadsafeFS &TFS, - const GlobalCompilationDatabase &CDB, - BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize, - std::function OnProgress, - std::function ContextProvider) + const ThreadsafeFS &TFS, const GlobalCompilationDatabase &CDB, + BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts) : SwapIndex(std::make_unique()), TFS(TFS), CDB(CDB), - BackgroundContext(std::move(BackgroundContext)), - ContextProvider(std::move(ContextProvider)), - Rebuilder(this, &IndexedSymbols, ThreadPoolSize), + ContextProvider(std::move(Opts.ContextProvider)), + Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize), IndexStorageFactory(std::move(IndexStorageFactory)), - Queue(std::move(OnProgress)), + Queue(std::move(Opts.OnProgress)), CommandsChanged( CDB.watch([&](const std::vector &ChangedFiles) { enqueue(ChangedFiles); })) { - assert(ThreadPoolSize > 0 && "Thread pool size can't be zero."); + assert(Opts.ThreadPoolSize > 0 && "Thread pool size can't be zero."); assert(this->IndexStorageFactory && "Storage factory can not be null!"); - for (unsigned I = 0; I < ThreadPoolSize; ++I) { - ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1), [this] { - WithContext Ctx(this->BackgroundContext.clone()); - Queue.work([&] { Rebuilder.idle(); }); - }); + for (unsigned I = 0; I < Opts.ThreadPoolSize; ++I) { + ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1), + [this, Ctx(Context::current().clone())]() mutable { + WithContext BGContext(std::move(Ctx)); + Queue.work([&] { Rebuilder.idle(); }); + }); } } diff --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h index 9adad173768637..72fe84466959fe 100644 --- a/clang-tools-extra/clangd/index/Background.h +++ b/clang-tools-extra/clangd/index/Background.h @@ -124,22 +124,26 @@ class BackgroundQueue { // Builds an in-memory index by by running the static indexer action over // all commands in a compilation database. Indexing happens in the background. -// FIXME: it should also persist its state on disk for fast start. // FIXME: it should watch for changes to files on disk. class BackgroundIndex : public SwapIndex { public: - /// If BuildIndexPeriodMs is greater than 0, the symbol index will only be - /// rebuilt periodically (one per \p BuildIndexPeriodMs); otherwise, index is - /// rebuilt for each indexed file. - BackgroundIndex( - Context BackgroundContext, const ThreadsafeFS &, - const GlobalCompilationDatabase &CDB, - BackgroundIndexStorage::Factory IndexStorageFactory, - // Arbitrary value to ensure some concurrency in tests. - // In production an explicit value is passed. - size_t ThreadPoolSize = 4, - std::function OnProgress = nullptr, - std::function ContextProvider = nullptr); + struct Options { + // Arbitrary value to ensure some concurrency in tests. + // In production an explicit value is specified. + size_t ThreadPoolSize = 4; + // Callback that provides notifications as indexing makes progress. + std::function OnProgress = nullptr; + // Function called to obtain the Context to use while indexing the specified + // file. Called with the empty string for other tasks. + // (When called, the context from BackgroundIndex construction is active). + std::function ContextProvider = nullptr; + }; + + /// Creates a new background index and starts its threads. + /// The current Context will be propagated to each worker thread. + BackgroundIndex(const ThreadsafeFS &, const GlobalCompilationDatabase &CDB, + BackgroundIndexStorage::Factory IndexStorageFactory, + Options Opts); ~BackgroundIndex(); // Blocks while the current task finishes. // Enqueue translation units for indexing. @@ -183,7 +187,6 @@ class BackgroundIndex : public SwapIndex { // configuration const ThreadsafeFS &TFS; const GlobalCompilationDatabase &CDB; - Context BackgroundContext; std::function ContextProvider; llvm::Error index(tooling::CompileCommand); diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index c163951aff9bae..a3ceaa388cf9db 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -20,6 +20,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" @@ -156,16 +157,21 @@ getTokenLocation(SourceLocation TokLoc, const SourceManager &SM, return Result; } -// Checks whether \p ND is a definition of a TagDecl (class/struct/enum/union) -// in a header file, in which case clangd would prefer to use ND as a canonical -// declaration. -// FIXME: handle symbol types that are not TagDecl (e.g. functions), if using -// the first seen declaration as canonical declaration is not a good enough -// heuristic. +// Checks whether \p ND is a good candidate to be the *canonical* declaration of +// its symbol (e.g. a go-to-declaration target). This overrides the default of +// using Clang's canonical declaration, which is the first in the TU. +// +// Example: preferring a class declaration over its forward declaration. bool isPreferredDeclaration(const NamedDecl &ND, index::SymbolRoleSet Roles) { const auto &SM = ND.getASTContext().getSourceManager(); - return (Roles & static_cast(index::SymbolRole::Definition)) && - isa(&ND) && !isInsideMainFile(ND.getLocation(), SM); + if (isa(ND)) + return (Roles & static_cast(index::SymbolRole::Definition)) && + !isInsideMainFile(ND.getLocation(), SM); + if (const auto *ID = dyn_cast(&ND)) + return ID->isThisDeclarationADefinition(); + if (const auto *PD = dyn_cast(&ND)) + return PD->isThisDeclarationADefinition(); + return false; } RefKind toRefKind(index::SymbolRoleSet Roles, bool Spelled = false) { @@ -267,6 +273,25 @@ bool SymbolCollector::handleDeclOccurrence( // picked a replacement for D if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) D = CanonicalDecls.try_emplace(D, ASTNode.OrigD).first->second; + // Flag to mark that D should be considered canonical meaning its declaration + // will override any previous declaration for the Symbol. + bool DeclIsCanonical = false; + // Avoid treating ObjCImplementationDecl as a canonical declaration if it has + // a corresponding non-implicit and non-forward declared ObjcInterfaceDecl. + if (const auto *IID = dyn_cast(D)) { + DeclIsCanonical = true; + if (const auto *CID = IID->getClassInterface()) + if (const auto *DD = CID->getDefinition()) + if (!DD->isImplicitInterfaceDecl()) + D = DD; + } + // Avoid treating ObjCCategoryImplDecl as a canonical declaration in favor of + // its ObjCCategoryDecl if it has one. + if (const auto *CID = dyn_cast(D)) { + DeclIsCanonical = true; + if (const auto *CD = CID->getCategoryDecl()) + D = CD; + } const NamedDecl *ND = dyn_cast(D); if (!ND) return true; @@ -331,14 +356,14 @@ bool SymbolCollector::handleDeclOccurrence( return true; const Symbol *BasicSymbol = Symbols.find(*ID); - if (!BasicSymbol) // Regardless of role, ND is the canonical declaration. - BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly); - else if (isPreferredDeclaration(*OriginalDecl, Roles)) - // If OriginalDecl is preferred, replace the existing canonical + if (isPreferredDeclaration(*OriginalDecl, Roles)) + // If OriginalDecl is preferred, replace/create the existing canonical // declaration (e.g. a class forward declaration). There should be at most // one duplicate as we expect to see only one preferred declaration per // TU, because in practice they are definitions. BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID), IsMainFileOnly); + else if (!BasicSymbol || DeclIsCanonical) + BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly); if (Roles & static_cast(index::SymbolRole::Definition)) addDefinition(*OriginalDecl, *BasicSymbol); diff --git a/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp b/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp index ca35f620bba184..7652dbb92d0202 100644 --- a/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp +++ b/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp @@ -30,10 +30,12 @@ llvm::cl::opt IndexLocation( llvm::cl::Positional); llvm::cl::opt - ExecCommand("c", llvm::cl::desc("Command to execute and then exit")); + ExecCommand("c", llvm::cl::desc("Command to execute and then exit.")); -llvm::cl::opt ProjectRoot("project-root", - llvm::cl::desc("Path to the project")); +llvm::cl::opt ProjectRoot( + "project-root", + llvm::cl::desc( + "Path to the project. Required when connecting using remote index.")); static constexpr char Overview[] = R"( This is an **experimental** interactive tool to process user-provided search @@ -373,10 +375,14 @@ int main(int argc, const char *argv[]) { llvm::cl::ResetCommandLineParser(); // We reuse it for REPL commands. llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); + bool RemoteMode = llvm::StringRef(IndexLocation).startswith("remote:"); + if (RemoteMode && ProjectRoot.empty()) { + llvm::errs() << "--project-root is required in remote mode\n"; + return -1; + } + std::unique_ptr Index; - reportTime(llvm::StringRef(IndexLocation).startswith("remote:") - ? "Remote index client creation" - : "Dex build", + reportTime(RemoteMode ? "Remote index client creation" : "Dex build", [&]() { Index = openIndex(IndexLocation); }); if (!Index) { diff --git a/clang-tools-extra/clangd/index/remote/Client.cpp b/clang-tools-extra/clangd/index/remote/Client.cpp index 4c1741e715a55f..131e4c0b2fce21 100644 --- a/clang-tools-extra/clangd/index/remote/Client.cpp +++ b/clang-tools-extra/clangd/index/remote/Client.cpp @@ -76,7 +76,9 @@ class IndexClient : public clangd::SymbolIndex { : Stub(remote::SymbolIndex::NewStub(Channel)), ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"", /*LocalIndexRoot=*/ProjectRoot)), - DeadlineWaitingTime(DeadlineTime) {} + DeadlineWaitingTime(DeadlineTime) { + assert(!ProjectRoot.empty()); + } void lookup(const clangd::LookupRequest &Request, llvm::function_ref Callback) const { diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp index dbda5bf24aa3b1..cfc72ce87be61b 100644 --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp @@ -59,14 +59,16 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot, assert(llvm::sys::path::is_absolute(RemoteIndexRoot)); assert(RemoteIndexRoot == llvm::sys::path::convert_to_slash(RemoteIndexRoot)); - assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator())); this->RemoteIndexRoot = RemoteIndexRoot.str(); + if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator())) + *this->RemoteIndexRoot += llvm::sys::path::get_separator(); } if (!LocalIndexRoot.empty()) { assert(llvm::sys::path::is_absolute(LocalIndexRoot)); assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot)); - assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator())); this->LocalIndexRoot = LocalIndexRoot.str(); + if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator())) + *this->LocalIndexRoot += llvm::sys::path::get_separator(); } assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty()); } diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp index 895afbb116f18d..d4c723e02eebec 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp @@ -673,6 +673,8 @@ bool ExtractFunction::prepare(const Selection &Inputs) { const Node *CommonAnc = Inputs.ASTSelection.commonAncestor(); const SourceManager &SM = Inputs.AST->getSourceManager(); const LangOptions &LangOpts = Inputs.AST->getLangOpts(); + if (!LangOpts.CPlusPlus) + return false; if (auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts)) { ExtZone = std::move(*MaybeExtZone); return true; diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index daf87d11c38438..3d83f3652f3003 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -682,7 +682,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var if (!ResourceDir.empty()) Opts.ResourceDir = ResourceDir; Opts.BuildDynamicSymbolIndex = EnableIndex; - Opts.BackgroundIndex = EnableBackgroundIndex; std::unique_ptr StaticIdx; std::future AsyncIndexLoad; // Block exit while loading the index. if (EnableIndex && !IndexFile.empty()) { @@ -713,6 +712,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var } } #endif + Opts.BackgroundIndex = EnableBackgroundIndex; Opts.StaticIndex = StaticIdx.get(); Opts.AsyncThreadsCount = WorkerThreadsCount; Opts.BuildRecoveryAST = RecoveryAST; @@ -808,23 +808,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var // FIXME: use the FS provided to the function. Opts = ClangTidyOptProvider->getOptions(File); } - if (!Opts.Checks) { - // If the user hasn't configured clang-tidy checks at all, including - // via .clang-tidy, give them a nice set of checks. - // (This should be what the "default" options does, but it isn't...) - // - // These default checks are chosen for: - // - low false-positive rate - // - providing a lot of value - // - being reasonably efficient - Opts.Checks = llvm::join_items( - ",", "readability-misleading-indentation", - "readability-deleted-default", "bugprone-integer-division", - "bugprone-sizeof-expression", "bugprone-suspicious-missing-comma", - "bugprone-unused-raii", "bugprone-unused-return-value", - "misc-unused-using-decls", "misc-unused-alias-decls", - "misc-definitions-in-headers"); - } return Opts; }; } diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp index f1c582ef1abe57..06614872363f54 100644 --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -96,8 +96,8 @@ TEST_F(BackgroundIndexTest, NoCrashOnErrorFile) { size_t CacheHits = 0; MemoryShardStorage MSS(Storage, CacheHits); OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); tooling::CompileCommand Cmd; Cmd.Filename = testPath("root/A.cc"); @@ -127,7 +127,8 @@ TEST_F(BackgroundIndexTest, Config) { // Context provider that installs a configuration mutating foo's command. // This causes it to define foo::two instead of foo::one. // It also disables indexing of baz entirely. - auto ContextProvider = [](PathRef P) { + BackgroundIndex::Options Opts; + Opts.ContextProvider = [](PathRef P) { Config C; if (P.endswith("foo.cpp")) C.CompileFlags.Edits.push_back( @@ -143,9 +144,9 @@ TEST_F(BackgroundIndexTest, Config) { // We need the CommandMangler, because that applies the config we're testing. OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{}, tooling::ArgumentsAdjuster(CommandMangler::forTests())); + BackgroundIndex Idx( - Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; }, - /*ThreadPoolSize=*/4, /*OnProgress=*/nullptr, std::move(ContextProvider)); + FS, CDB, [&](llvm::StringRef) { return &MSS; }, std::move(Opts)); // Index the two files. for (auto &Cmd : Cmds) { std::string FullPath = testPath(Cmd.Filename); @@ -186,8 +187,8 @@ TEST_F(BackgroundIndexTest, IndexTwoFiles) { size_t CacheHits = 0; MemoryShardStorage MSS(Storage, CacheHits); OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); tooling::CompileCommand Cmd; Cmd.Filename = testPath("root/A.cc"); @@ -253,8 +254,8 @@ TEST_F(BackgroundIndexTest, ShardStorageTest) { // Check nothing is loaded from Storage, but A.cc and A.h has been stored. { OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -263,8 +264,8 @@ TEST_F(BackgroundIndexTest, ShardStorageTest) { { OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -321,8 +322,8 @@ TEST_F(BackgroundIndexTest, DirectIncludesTest) { Cmd.CommandLine = {"clang++", testPath("root/A.cc")}; { OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -371,8 +372,8 @@ TEST_F(BackgroundIndexTest, ShardStorageLoad) { // Check nothing is loaded from Storage, but A.cc and A.h has been stored. { OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -386,8 +387,8 @@ TEST_F(BackgroundIndexTest, ShardStorageLoad) { )cpp"; { OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -404,8 +405,8 @@ TEST_F(BackgroundIndexTest, ShardStorageLoad) { { CacheHits = 0; OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -445,8 +446,8 @@ TEST_F(BackgroundIndexTest, ShardStorageEmptyFile) { // Check that A.cc, A.h and B.h has been stored. { OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -461,8 +462,8 @@ TEST_F(BackgroundIndexTest, ShardStorageEmptyFile) { { CacheHits = 0; OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -477,8 +478,8 @@ TEST_F(BackgroundIndexTest, ShardStorageEmptyFile) { { CacheHits = 0; OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } @@ -495,8 +496,8 @@ TEST_F(BackgroundIndexTest, NoDotsInAbsPath) { size_t CacheHits = 0; MemoryShardStorage MSS(Storage, CacheHits); OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); ASSERT_TRUE(Idx.blockUntilIdleForTest()); tooling::CompileCommand Cmd; @@ -526,8 +527,8 @@ TEST_F(BackgroundIndexTest, UncompilableFiles) { size_t CacheHits = 0; MemoryShardStorage MSS(Storage, CacheHits); OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); tooling::CompileCommand Cmd; FS.Files[testPath("A.h")] = "void foo();"; @@ -589,8 +590,8 @@ TEST_F(BackgroundIndexTest, CmdLineHash) { size_t CacheHits = 0; MemoryShardStorage MSS(Storage, CacheHits); OverlayCDB CDB(/*Base=*/nullptr); - BackgroundIndex Idx(Context::empty(), FS, CDB, - [&](llvm::StringRef) { return &MSS; }); + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*Opts=*/{}); tooling::CompileCommand Cmd; FS.Files[testPath("A.cc")] = "#include \"A.h\""; diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index c25e2b7f810373..966fa9630852b6 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -63,6 +63,7 @@ add_unittest(ClangdUnitTests ClangdTests IndexTests.cpp JSONTransportTests.cpp LSPClient.cpp + ModulesTests.cpp ParsedASTTests.cpp PathMappingTests.cpp PreambleTests.cpp diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp index c21719d58bd6ad..813b95aa3c824b 100644 --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -15,6 +15,7 @@ #include "Matchers.h" #include "SyncAPI.h" #include "TestFS.h" +#include "TestTU.h" #include "URI.h" #include "support/Path.h" #include "support/Threading.h" @@ -25,9 +26,12 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Errc.h" #include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" +#include "llvm/Support/VirtualFileSystem.h" +#include "llvm/Testing/Support/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include @@ -137,41 +141,54 @@ std::string replacePtrsInDump(std::string const &Dump) { return Result; } +std::string dumpAST(ClangdServer &Server, PathRef File) { + std::string Result; + Notification Done; + Server.customAction(File, "DumpAST", [&](llvm::Expected AST) { + if (AST) { + llvm::raw_string_ostream ResultOS(Result); + AST->AST.getASTContext().getTranslationUnitDecl()->dump(ResultOS, true); + } else { + llvm::consumeError(AST.takeError()); + Result = ""; + } + Done.notify(); + }); + Done.wait(); + return Result; +} + std::string dumpASTWithoutMemoryLocs(ClangdServer &Server, PathRef File) { - auto DumpWithMemLocs = runDumpAST(Server, File); - return replacePtrsInDump(DumpWithMemLocs); + return replacePtrsInDump(dumpAST(Server, File)); } -class ClangdVFSTest : public ::testing::Test { -protected: - std::string parseSourceAndDumpAST( - PathRef SourceFileRelPath, llvm::StringRef SourceContents, - std::vector> ExtraFiles = {}, - bool ExpectErrors = false) { - MockFS FS; - ErrorCheckingCallbacks DiagConsumer; - MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, ClangdServer::optsForTest(), &DiagConsumer); - for (const auto &FileWithContents : ExtraFiles) - FS.Files[testPath(FileWithContents.first)] = - std::string(FileWithContents.second); - - auto SourceFilename = testPath(SourceFileRelPath); - Server.addDocument(SourceFilename, SourceContents); - auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename); - EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; - EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags()); - return Result; - } -}; +std::string parseSourceAndDumpAST( + PathRef SourceFileRelPath, llvm::StringRef SourceContents, + std::vector> ExtraFiles = {}, + bool ExpectErrors = false) { + MockFS FS; + ErrorCheckingCallbacks DiagConsumer; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, FS, ClangdServer::optsForTest(), &DiagConsumer); + for (const auto &FileWithContents : ExtraFiles) + FS.Files[testPath(FileWithContents.first)] = + std::string(FileWithContents.second); + + auto SourceFilename = testPath(SourceFileRelPath); + Server.addDocument(SourceFilename, SourceContents); + auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename); + EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; + EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags()); + return Result; +} -TEST_F(ClangdVFSTest, Parse) { +TEST(ClangdServerTest, Parse) { // FIXME: figure out a stable format for AST dumps, so that we can check the // output of the dump itself is equal to the expected one, not just that it's // different. - auto Empty = parseSourceAndDumpAST("foo.cpp", "", {}); - auto OneDecl = parseSourceAndDumpAST("foo.cpp", "int a;", {}); - auto SomeDecls = parseSourceAndDumpAST("foo.cpp", "int a; int b; int c;", {}); + auto Empty = parseSourceAndDumpAST("foo.cpp", ""); + auto OneDecl = parseSourceAndDumpAST("foo.cpp", "int a;"); + auto SomeDecls = parseSourceAndDumpAST("foo.cpp", "int a; int b; int c;"); EXPECT_NE(Empty, OneDecl); EXPECT_NE(Empty, SomeDecls); EXPECT_NE(SomeDecls, OneDecl); @@ -184,7 +201,7 @@ TEST_F(ClangdVFSTest, Parse) { EXPECT_EQ(SomeDecls, SomeDecls2); } -TEST_F(ClangdVFSTest, ParseWithHeader) { +TEST(ClangdServerTest, ParseWithHeader) { parseSourceAndDumpAST("foo.cpp", "#include \"foo.h\"", {}, /*ExpectErrors=*/true); parseSourceAndDumpAST("foo.cpp", "#include \"foo.h\"", {{"foo.h", ""}}, @@ -200,7 +217,7 @@ int b = a; /*ExpectErrors=*/false); } -TEST_F(ClangdVFSTest, Reparse) { +TEST(ClangdServerTest, Reparse) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -235,7 +252,7 @@ int b = a; EXPECT_NE(DumpParse1, DumpParseEmpty); } -TEST_F(ClangdVFSTest, ReparseOnHeaderChange) { +TEST(ClangdServerTest, ReparseOnHeaderChange) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -273,7 +290,7 @@ int b = a; EXPECT_NE(DumpParse1, DumpParseDifferent); } -TEST_F(ClangdVFSTest, PropagatesContexts) { +TEST(ClangdServerTest, PropagatesContexts) { static Key Secret; struct ContextReadingFS : public ThreadsafeFS { mutable int Got; @@ -346,7 +363,7 @@ TEST(ClangdServerTest, RespectsConfig) { EXPECT_NE(Result->front().PreferredDeclaration.range, Example.range()); } -TEST_F(ClangdVFSTest, PropagatesVersion) { +TEST(ClangdServerTest, PropagatesVersion) { MockCompilationDatabase CDB; MockFS FS; struct Callbacks : public ClangdServer::Callbacks { @@ -365,7 +382,7 @@ TEST_F(ClangdVFSTest, PropagatesVersion) { // Only enable this test on Unix #ifdef LLVM_ON_UNIX -TEST_F(ClangdVFSTest, SearchLibDir) { +TEST(ClangdServerTest, SearchLibDir) { // Checks that searches for GCC installation is done through vfs. MockFS FS; ErrorCheckingCallbacks DiagConsumer; @@ -415,7 +432,7 @@ std::string x; } #endif // LLVM_ON_UNIX -TEST_F(ClangdVFSTest, ForceReparseCompileCommand) { +TEST(ClangdServerTest, ForceReparseCompileCommand) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -451,7 +468,7 @@ struct bar { T x; }; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); } -TEST_F(ClangdVFSTest, ForceReparseCompileCommandDefines) { +TEST(ClangdServerTest, ForceReparseCompileCommandDefines) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -483,7 +500,7 @@ int main() { return 0; } } // Test ClangdServer.reparseOpenedFiles. -TEST_F(ClangdVFSTest, ReparseOpenedFiles) { +TEST(ClangdServerTest, ReparseOpenedFiles) { Annotations FooSource(R"cpp( #ifdef MACRO static void $one[[bob]]() {} @@ -553,7 +570,7 @@ MATCHER_P4(Stats, Name, UsesMemory, PreambleBuilds, ASTBuilds, "") { std::tie(PreambleBuilds, ASTBuilds); } -TEST_F(ClangdVFSTest, FileStats) { +TEST(ClangdServerTest, FileStats) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -589,7 +606,7 @@ struct Something { EXPECT_THAT(Server.fileStats(), IsEmpty()); } -TEST_F(ClangdVFSTest, InvalidCompileCommand) { +TEST(ClangdServerTest, InvalidCompileCommand) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -605,7 +622,7 @@ TEST_F(ClangdVFSTest, InvalidCompileCommand) { // Clang can't parse command args in that case, but we shouldn't crash. runAddDocument(Server, FooCpp, "int main() {}"); - EXPECT_EQ(runDumpAST(Server, FooCpp), ""); + EXPECT_EQ(dumpAST(Server, FooCpp), ""); EXPECT_ERROR(runLocateSymbolAt(Server, FooCpp, Position())); EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position())); EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name", @@ -619,9 +636,7 @@ TEST_F(ClangdVFSTest, InvalidCompileCommand) { Field(&CodeCompletion::Name, "main"))); } -class ClangdThreadingTest : public ClangdVFSTest {}; - -TEST_F(ClangdThreadingTest, StressTest) { +TEST(ClangdThreadingTest, StressTest) { // Without 'static' clang gives an error for a usage inside TestDiagConsumer. static const unsigned FilesCount = 5; const unsigned RequestsCount = 500; @@ -839,7 +854,7 @@ int d; } } -TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) { +TEST(ClangdThreadingTest, NoConcurrentDiagnostics) { class NoConcurrentAccessDiagConsumer : public ClangdServer::Callbacks { public: std::atomic Count = {0}; @@ -901,7 +916,7 @@ int d; ASSERT_EQ(DiagConsumer.Count, 2); // Sanity check - we actually ran both? } -TEST_F(ClangdVFSTest, FormatCode) { +TEST(ClangdServerTest, FormatCode) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -930,7 +945,7 @@ void f() {} EXPECT_EQ(Expected, *Changed); } -TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) { +TEST(ClangdServerTest, ChangedHeaderFromISystem) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -1036,7 +1051,7 @@ TEST(ClangdTests, PreambleVFSStatCache) { } #endif -TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) { +TEST(ClangdServerTest, FallbackWhenPreambleIsNotReady) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -1082,7 +1097,7 @@ TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) { Field(&CodeCompletion::Scope, "")))); } -TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) { +TEST(ClangdServerTest, FallbackWhenWaitingForCompileCommand) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; // Returns compile command only when notified. @@ -1141,10 +1156,25 @@ TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) { Field(&CodeCompletion::Scope, "ns::")))); } +TEST(ClangdServerTest, CustomAction) { + OverlayCDB CDB(/*Base=*/nullptr); + MockFS FS; + ClangdServer Server(CDB, FS, ClangdServer::optsForTest()); + + Server.addDocument(testPath("foo.cc"), "void x();"); + Decl::Kind XKind = Decl::TranslationUnit; + EXPECT_THAT_ERROR(runCustomAction(Server, testPath("foo.cc"), + [&](InputsAndAST AST) { + XKind = findDecl(AST.AST, "x").getKind(); + }), + llvm::Succeeded()); + EXPECT_EQ(XKind, Decl::Function); +} + // Tests fails when built with asan due to stack overflow. So skip running the // test as a workaround. #if !defined(__has_feature) || !__has_feature(address_sanitizer) -TEST_F(ClangdVFSTest, TestStackOverflow) { +TEST(ClangdServerTest, TestStackOverflow) { MockFS FS; ErrorCheckingCallbacks DiagConsumer; MockCompilationDatabase CDB; @@ -1166,6 +1196,44 @@ TEST_F(ClangdVFSTest, TestStackOverflow) { } #endif +TEST(ClangdServer, TidyOverrideTest) { + struct DiagsCheckingCallback : public ClangdServer::Callbacks { + public: + void onDiagnosticsReady(PathRef File, llvm::StringRef Version, + std::vector Diagnostics) override { + std::lock_guard Lock(Mutex); + HadDiagsInLastCallback = !Diagnostics.empty(); + } + + std::mutex Mutex; + bool HadDiagsInLastCallback = false; + } DiagConsumer; + + MockFS FS; + MockCompilationDatabase CDB; + CDB.ExtraClangFlags = {"-xc++"}; + auto Opts = ClangdServer::optsForTest(); + Opts.GetClangTidyOptions = [](llvm::vfs::FileSystem &, llvm::StringRef) { + auto Opts = tidy::ClangTidyOptions::getDefaults(); + // These checks don't work well in clangd, even if configured they shouldn't + // run. + Opts.Checks = "bugprone-use-after-move,llvm-header-guard"; + return Opts; + }; + ClangdServer Server(CDB, FS, Opts, &DiagConsumer); + const char *SourceContents = R"cpp( + struct Foo { Foo(); Foo(Foo&); Foo(Foo&&); }; + namespace std { Foo&& move(Foo&); } + void foo() { + Foo x; + Foo y = std::move(x); + Foo z = x; + })cpp"; + Server.addDocument(testPath("foo.h"), SourceContents); + ASSERT_TRUE(Server.blockUntilIdleForTest()); + EXPECT_FALSE(DiagConsumer.HadDiagsInLastCallback); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 31a1c2d46c09b9..635e036039a016 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -194,9 +194,14 @@ TEST(CompletionTest, Filter) { EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions, AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux")))); - // Macros require prefix match. - EXPECT_THAT(completions(Body + "int main() { C^ }").Completions, - AllOf(Has("Car"), Not(Has("MotorCar")))); + // Macros require prefix match, either from index or AST. + Symbol Sym = var("MotorCarIndex"); + Sym.SymInfo.Kind = index::SymbolKind::Macro; + EXPECT_THAT( + completions(Body + "int main() { C^ }", {Sym}).Completions, + AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex")))); + EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions, + AllOf(Has("MotorCar"), Has("MotorCarIndex"))); } void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) { @@ -1161,46 +1166,75 @@ TEST(SignatureHelpTest, ActiveArg) { } TEST(SignatureHelpTest, OpeningParen) { - llvm::StringLiteral Tests[] = {// Recursive function call. - R"cpp( - int foo(int a, int b, int c); - int main() { - foo(foo $p^( foo(10, 10, 10), ^ ))); - })cpp", - // Functional type cast. - R"cpp( - struct Foo { - Foo(int a, int b, int c); - }; - int main() { - Foo $p^( 10, ^ ); - })cpp", - // New expression. - R"cpp( - struct Foo { - Foo(int a, int b, int c); - }; - int main() { - new Foo $p^( 10, ^ ); - })cpp", - // Macro expansion. - R"cpp( - int foo(int a, int b, int c); - #define FOO foo( - - int main() { - // Macro expansions. - $p^FOO 10, ^ ); - })cpp", - // Macro arguments. - R"cpp( - int foo(int a, int b, int c); - int main() { - #define ID(X) X - // FIXME: figure out why ID(foo (foo(10), )) doesn't work when preserving - // the recovery expression. - ID(foo $p^( 10, ^ )) - })cpp"}; + llvm::StringLiteral Tests[] = { + // Recursive function call. + R"cpp( + int foo(int a, int b, int c); + int main() { + foo(foo $p^( foo(10, 10, 10), ^ ))); + })cpp", + // Functional type cast. + R"cpp( + struct Foo { + Foo(int a, int b, int c); + }; + int main() { + Foo $p^( 10, ^ ); + })cpp", + // New expression. + R"cpp( + struct Foo { + Foo(int a, int b, int c); + }; + int main() { + new Foo $p^( 10, ^ ); + })cpp", + // Macro expansion. + R"cpp( + int foo(int a, int b, int c); + #define FOO foo( + + int main() { + // Macro expansions. + $p^FOO 10, ^ ); + })cpp", + // Macro arguments. + R"cpp( + int foo(int a, int b, int c); + int main() { + #define ID(X) X + // FIXME: figure out why ID(foo (foo(10), )) doesn't work when preserving + // the recovery expression. + ID(foo $p^( 10, ^ )) + })cpp", + // Dependent args. + R"cpp( + int foo(int a, int b); + template void bar(T t) { + foo$p^(t, ^t); + })cpp", + // Dependent args on templated func. + R"cpp( + template + int foo(T, T); + template void bar(T t) { + foo$p^(t, ^t); + })cpp", + // Dependent args on member. + R"cpp( + struct Foo { int foo(int, int); }; + template void bar(T t) { + Foo f; + f.foo$p^(t, ^t); + })cpp", + // Dependent args on templated member. + R"cpp( + struct Foo { template int foo(T, T); }; + template void bar(T t) { + Foo f; + f.foo$p^(t, ^t); + })cpp", + }; for (auto Test : Tests) { Annotations Code(Test); diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index e8757079c6752d..6974131562b32a 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -29,6 +29,8 @@ namespace clangd { namespace { using ::testing::_; +using ::testing::AllOf; +using ::testing::Contains; using ::testing::ElementsAre; using ::testing::Field; using ::testing::IsEmpty; @@ -278,6 +280,23 @@ TEST(DiagnosticsTest, ClangTidy) { "use a trailing return type for this function"))))); } +TEST(DiagnosticsTest, ClangTidyEOF) { + // clang-format off + Annotations Test(R"cpp( + [[#]]include + #include "a.h")cpp"); + // clang-format on + auto TU = TestTU::withCode(Test.code()); + TU.ExtraArgs = {"-isystem."}; + TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = ""; + TU.ClangTidyChecks = "-*, llvm-include-order"; + EXPECT_THAT( + TU.build().getDiagnostics(), + Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"), + DiagSource(Diag::ClangTidy), + DiagName("llvm-include-order")))); +} + TEST(DiagnosticTest, TemplatesInHeaders) { // Diagnostics from templates defined in headers are placed at the expansion. Annotations Main(R"cpp( diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 3421b9cec2d307..4c655c3338d203 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -376,6 +376,15 @@ TEST_F(TargetDeclTest, ClassTemplate) { {"template<> class Foo", Rel::TemplateInstantiation}, {"template class Foo", Rel::TemplatePattern}); + Code = R"cpp( + // Template template argument. + template struct Vector {}; + template