diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -123,7 +123,10 @@ /// Returns the name of the clang-tidy check which produced this /// diagnostic ID. - std::string getCheckName(unsigned DiagnosticID) const; + static StringRef getDiagnosticName(const Diagnostic &Info, + SmallVectorImpl &Buffer); + + static std::string getDiagnosticName(const Diagnostic &Info); /// Returns \c true if the check is enabled for the \c CurrentFile. /// @@ -204,8 +207,6 @@ std::string CurrentBuildDirectory; - llvm::DenseMap CheckNamesByDiagnosticID; - bool Profile; std::string ProfilePrefix; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -30,6 +30,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Regex.h" #include @@ -50,14 +51,6 @@ DiagnosticsEngine::Level Level, StringRef Message, ArrayRef Ranges, DiagOrStoredDiag Info) override { - // Remove check name from the message. - // FIXME: Remove this once there's a better way to pass check names than - // appending the check name to the message in ClangTidyContext::diag and - // using getCustomDiagID. - std::string CheckNameInMessage = " [" + Error.DiagnosticName + "]"; - if (Message.endswith(CheckNameInMessage)) - Message = Message.substr(0, Message.size() - CheckNameInMessage.size()); - auto TidyMessage = Loc.isValid() ? tooling::DiagnosticMessage(Message, Loc.getManager(), Loc) @@ -165,23 +158,30 @@ ClangTidyContext::~ClangTidyContext() = default; +static constexpr llvm::StringLiteral CheckPrefix(":"); + DiagnosticBuilder ClangTidyContext::diag( StringRef CheckName, SourceLocation Loc, StringRef Description, DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) { assert(Loc.isValid()); - unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID( - Level, (Description + " [" + CheckName + "]").str()); - CheckNamesByDiagnosticID.try_emplace(ID, CheckName); - return DiagEngine->Report(Loc, ID); + unsigned ID = + DiagEngine->getDiagnosticIDs()->getCustomDiagID(Level, Description); + auto Res = DiagEngine->Report(Loc, ID); + // Repurpse the Flag value to store the check name. clang-tidy checks don't + // have command line flags so this field is safe to use, but add a prefix just + // to be sure. + Res << AddFlagValue(SmallString<64>{CheckPrefix, CheckName}); + return Res; } DiagnosticBuilder ClangTidyContext::diag( StringRef CheckName, StringRef Description, DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) { - unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID( - Level, (Description + " [" + CheckName + "]").str()); - CheckNamesByDiagnosticID.try_emplace(ID, CheckName); - return DiagEngine->Report(ID); + unsigned ID = + DiagEngine->getDiagnosticIDs()->getCustomDiagID(Level, Description); + auto Res = DiagEngine->Report(ID); + Res.addFlagValue(SmallString<64>({CheckPrefix, CheckName})); + return Res; } DiagnosticBuilder ClangTidyContext::configurationDiag( @@ -246,18 +246,30 @@ return WarningAsErrorFilter->contains(CheckName); } -std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const { - std::string ClangWarningOption = std::string( - DiagEngine->getDiagnosticIDs()->getWarningOptionForDiag(DiagnosticID)); - if (!ClangWarningOption.empty()) - return "clang-diagnostic-" + ClangWarningOption; - llvm::DenseMap::const_iterator I = - CheckNamesByDiagnosticID.find(DiagnosticID); - if (I != CheckNamesByDiagnosticID.end()) - return I->second; +StringRef ClangTidyContext::getDiagnosticName(const Diagnostic &Info, + SmallVectorImpl &Buffer) { + auto CheckName = Info.getDiags()->getFlagValue(); + if (!CheckName.empty() && CheckName.consume_front(CheckPrefix)) { + return CheckName; + } + StringRef ClangWarningOption = + DiagnosticIDs::getWarningOptionForDiag(Info.getID()); + if (!ClangWarningOption.empty()) { + constexpr llvm::StringLiteral Prefix("clang-diagnostic-"); + Buffer.resize_for_overwrite(Prefix.size() + ClangWarningOption.size()); + memcpy(Buffer.data(), Prefix.data(), Prefix.size()); + memcpy(Buffer.data() + Prefix.size(), ClangWarningOption.data(), + ClangWarningOption.size()); + return {Buffer.begin(), Buffer.size()}; + } return ""; } +std::string ClangTidyContext::getDiagnosticName(const Diagnostic &Info) { + SmallString<64> Buffer; + return getDiagnosticName(Info, Buffer).str(); +} + ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer( ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine, bool RemoveIncompatibleErrors) @@ -290,7 +302,8 @@ } static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line, - unsigned DiagID, const ClangTidyContext &Context) { + StringRef CheckName, + const ClangTidyContext &Context) { const size_t NolintIndex = Line.find(NolintDirectiveText); if (NolintIndex == StringRef::npos) return false; @@ -305,7 +318,6 @@ Line.substr(BracketIndex, BracketEndIndex - BracketIndex); // Allow disabling all the checks with "*". if (ChecksStr != "*") { - std::string CheckName = Context.getCheckName(DiagID); // Allow specifying a few check names, delimited with comma. SmallVector Checks; ChecksStr.split(Checks, ',', -1, false); @@ -325,7 +337,7 @@ } static bool LineIsMarkedWithNOLINT(const SourceManager &SM, SourceLocation Loc, - unsigned DiagID, + StringRef CheckName, const ClangTidyContext &Context, bool AllowIO) { FileID File; @@ -337,21 +349,22 @@ // Check if there's a NOLINT on this line. StringRef RestOfLine = Buffer->substr(Offset).split('\n').first; - if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context)) + if (IsNOLINTFound("NOLINT", RestOfLine, CheckName, Context)) return true; // Check if there's a NOLINTNEXTLINE on the previous line. StringRef PrevLine = Buffer->substr(0, Offset).rsplit('\n').first.rsplit('\n').second; - return IsNOLINTFound("NOLINTNEXTLINE", PrevLine, DiagID, Context); + return IsNOLINTFound("NOLINTNEXTLINE", PrevLine, CheckName, Context); } static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM, - SourceLocation Loc, unsigned DiagID, + SourceLocation Loc, + StringRef CheckName, const ClangTidyContext &Context, bool AllowIO) { while (true) { - if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context, AllowIO)) + if (LineIsMarkedWithNOLINT(SM, Loc, CheckName, Context, AllowIO)) return true; if (!Loc.isMacroID()) return false; @@ -366,12 +379,14 @@ bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, ClangTidyContext &Context, bool AllowIO) { + SmallString<64> Buffer; return Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && DiagLevel != DiagnosticsEngine::Fatal && - LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(), - Info.getLocation(), Info.getID(), - Context, AllowIO); + LineIsMarkedWithNOLINTinMacro( + Info.getSourceManager(), Info.getLocation(), + ClangTidyContext::getDiagnosticName(Info, Buffer), Context, + AllowIO); } } // namespace tidy @@ -398,7 +413,9 @@ "A diagnostic note can only be appended to a message."); } else { finalizeLastError(); - std::string CheckName = Context.getCheckName(Info.getID()); + SmallString<64> Buffer; + llvm::StringRef CheckName = + ClangTidyContext::getDiagnosticName(Info, Buffer); if (CheckName.empty()) { // This is a compiler diagnostic without a warning option. Assign check // name based on its level. diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -517,24 +517,8 @@ continue; } if (Tidy != nullptr) { - std::string TidyDiag = Tidy->getCheckName(Diag.ID); - if (!TidyDiag.empty()) { - Diag.Name = std::move(TidyDiag); + if (!Diag.Name.empty()) { Diag.Source = Diag::ClangTidy; - // clang-tidy bakes the name into diagnostic messages. Strip it out. - // It would be much nicer to make clang-tidy not do this. - auto CleanMessage = [&](std::string &Msg) { - StringRef Rest(Msg); - if (Rest.consume_back("]") && Rest.consume_back(Diag.Name) && - Rest.consume_back(" [")) - Msg.resize(Rest.size()); - }; - CleanMessage(Diag.Message); - for (auto &Note : Diag.Notes) - CleanMessage(Note.Message); - for (auto &Fix : Diag.Fixes) - CleanMessage(Fix.Message); - continue; } } } @@ -624,6 +608,7 @@ LastDiagLoc.reset(); LastDiagOriginallyError = OriginallyError; LastDiag->ID = Info.getID(); + LastDiag->Name = tidy::ClangTidyContext::getDiagnosticName(Info); fillNonLocationData(DiagLevel, Info, *LastDiag); LastDiag->InsideMainFile = true; // Put it at the start of the main file, for a lack of a better place. @@ -734,6 +719,7 @@ LastDiag = Diag(); FillDiagBase(*LastDiag); + LastDiag->Name = tidy::ClangTidyContext::getDiagnosticName(Info); LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager()); LastDiagOriginallyError = OriginallyError; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -323,9 +323,10 @@ isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress)) return DiagnosticsEngine::Ignored; if (!CTChecks.empty()) { - std::string CheckName = CTContext->getCheckName(Info.getID()); - bool IsClangTidyDiag = !CheckName.empty(); - if (IsClangTidyDiag) { + SmallString<64> Buffer; + auto CheckName = + tidy::ClangTidyContext::getDiagnosticName(Info, Buffer); + if (!CheckName.empty()) { if (Cfg.Diagnostics.Suppress.contains(CheckName)) return DiagnosticsEngine::Ignored; // Check for suppression comment. Skip the check for diagnostics not