Author: Karl-Johan Karlsson Date: 2025-01-17T10:23:27+01:00 New Revision: 73478708839fad8b02b3cfc84959d64a15ba93ca
URL: https://github.com/llvm/llvm-project/commit/73478708839fad8b02b3cfc84959d64a15ba93ca DIFF: https://github.com/llvm/llvm-project/commit/73478708839fad8b02b3cfc84959d64a15ba93ca.diff LOG: [diagtool] Make the BuiltinDiagnosticsByID table sorted (#120321) When building with -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON with a recent libstdc++ (e.g. from gcc 13.3.0) the testcase clang/test/Misc/warning-flags-tree.c fail with the message: ``` + diagtool tree --internal .../include/c++/13.3.0/bits/stl_algo.h:2013: In function: _ForwardIterator std::lower_bound(_ForwardIterator, _ForwardIterator, const _Tp &, _Compare) [_ForwardIterator = const diagtool::DiagnosticRecord *, _Tp = diagtool::DiagnosticRecord, _Compare = bool (*)(const diagtool::DiagnosticRecord &, const diagtool::DiagnosticRecord &)] Error: elements in iterator range [first, last) are not partitioned by the predicate __comp and value __val. Objects involved in the operation: iterator "first" @ 0x7ffea8ef2fd8 { } iterator "last" @ 0x7ffea8ef2fd0 { } ``` The reason for this error is that std::lower_bound is called on BuiltinDiagnosticsByID without it being entirely sorted. Calling std::lower_bound If the range is not sorted, the behavior of this function is undefined. This is detected when building with expensive checks. To make BuiltinDiagnosticsByID sorted we need to slightly change the order the inc-files are included. The include of DiagnosticCrossTUKinds.inc in DiagnosticNames.cpp is included too early and should be moved down directly after DiagnosticCommentKinds.inc. As a part of pull request the includes that build up BuiltinDiagnosticsByID table are extracted into a common wrapper header file AllDiagnosticKinds.inc that is used by both clang and diagtool. Added: clang/include/clang/Basic/AllDiagnosticKinds.inc Modified: clang/lib/Basic/DiagnosticIDs.cpp clang/tools/diagtool/DiagnosticNames.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/AllDiagnosticKinds.inc b/clang/include/clang/Basic/AllDiagnosticKinds.inc new file mode 100644 index 00000000000000..a946b4a640ac61 --- /dev/null +++ b/clang/include/clang/Basic/AllDiagnosticKinds.inc @@ -0,0 +1,33 @@ +//===--- AllDiagnosticKinds.inc----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// Defines the Diagnostic IDs in ID sorted order. The order is dictated by +/// the enum in DiagnosticIDs.h#L49-L65. +/// +//===----------------------------------------------------------------------===// + +// Turn off clang-format, as the order of the includes are important to make +// sure tables based on Diagnostic IDs are partitioned/sorted based on +// DiagID. + +// clang-format off +#include "clang/Basic/DiagnosticCommonKinds.inc" +#include "clang/Basic/DiagnosticDriverKinds.inc" +#include "clang/Basic/DiagnosticFrontendKinds.inc" +#include "clang/Basic/DiagnosticSerializationKinds.inc" +#include "clang/Basic/DiagnosticLexKinds.inc" +#include "clang/Basic/DiagnosticParseKinds.inc" +#include "clang/Basic/DiagnosticASTKinds.inc" +#include "clang/Basic/DiagnosticCommentKinds.inc" +#include "clang/Basic/DiagnosticCrossTUKinds.inc" +#include "clang/Basic/DiagnosticSemaKinds.inc" +#include "clang/Basic/DiagnosticAnalysisKinds.inc" +#include "clang/Basic/DiagnosticRefactoringKinds.inc" +#include "clang/Basic/DiagnosticInstallAPIKinds.inc" +// clang-format on diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index d77f28c80b2eb2..81194bbf2538e6 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -37,21 +37,7 @@ struct StaticDiagInfoDescriptionStringTable { #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \ SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY) \ char ENUM##_desc[sizeof(DESC)]; - // clang-format off -#include "clang/Basic/DiagnosticCommonKinds.inc" -#include "clang/Basic/DiagnosticDriverKinds.inc" -#include "clang/Basic/DiagnosticFrontendKinds.inc" -#include "clang/Basic/DiagnosticSerializationKinds.inc" -#include "clang/Basic/DiagnosticLexKinds.inc" -#include "clang/Basic/DiagnosticParseKinds.inc" -#include "clang/Basic/DiagnosticASTKinds.inc" -#include "clang/Basic/DiagnosticCommentKinds.inc" -#include "clang/Basic/DiagnosticCrossTUKinds.inc" -#include "clang/Basic/DiagnosticSemaKinds.inc" -#include "clang/Basic/DiagnosticAnalysisKinds.inc" -#include "clang/Basic/DiagnosticRefactoringKinds.inc" -#include "clang/Basic/DiagnosticInstallAPIKinds.inc" - // clang-format on +#include "clang/Basic/AllDiagnosticKinds.inc" #undef DIAG }; @@ -59,21 +45,7 @@ const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = { #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \ SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY) \ DESC, -// clang-format off -#include "clang/Basic/DiagnosticCommonKinds.inc" -#include "clang/Basic/DiagnosticDriverKinds.inc" -#include "clang/Basic/DiagnosticFrontendKinds.inc" -#include "clang/Basic/DiagnosticSerializationKinds.inc" -#include "clang/Basic/DiagnosticLexKinds.inc" -#include "clang/Basic/DiagnosticParseKinds.inc" -#include "clang/Basic/DiagnosticASTKinds.inc" -#include "clang/Basic/DiagnosticCommentKinds.inc" -#include "clang/Basic/DiagnosticCrossTUKinds.inc" -#include "clang/Basic/DiagnosticSemaKinds.inc" -#include "clang/Basic/DiagnosticAnalysisKinds.inc" -#include "clang/Basic/DiagnosticRefactoringKinds.inc" -#include "clang/Basic/DiagnosticInstallAPIKinds.inc" -// clang-format on +#include "clang/Basic/AllDiagnosticKinds.inc" #undef DIAG }; @@ -85,21 +57,7 @@ const uint32_t StaticDiagInfoDescriptionOffsets[] = { #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \ SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY) \ offsetof(StaticDiagInfoDescriptionStringTable, ENUM##_desc), -// clang-format off -#include "clang/Basic/DiagnosticCommonKinds.inc" -#include "clang/Basic/DiagnosticDriverKinds.inc" -#include "clang/Basic/DiagnosticFrontendKinds.inc" -#include "clang/Basic/DiagnosticSerializationKinds.inc" -#include "clang/Basic/DiagnosticLexKinds.inc" -#include "clang/Basic/DiagnosticParseKinds.inc" -#include "clang/Basic/DiagnosticASTKinds.inc" -#include "clang/Basic/DiagnosticCommentKinds.inc" -#include "clang/Basic/DiagnosticCrossTUKinds.inc" -#include "clang/Basic/DiagnosticSemaKinds.inc" -#include "clang/Basic/DiagnosticAnalysisKinds.inc" -#include "clang/Basic/DiagnosticRefactoringKinds.inc" -#include "clang/Basic/DiagnosticInstallAPIKinds.inc" -// clang-format on +#include "clang/Basic/AllDiagnosticKinds.inc" #undef DIAG }; diff --git a/clang/tools/diagtool/DiagnosticNames.cpp b/clang/tools/diagtool/DiagnosticNames.cpp index eb90f082437b33..c3a3002889c736 100644 --- a/clang/tools/diagtool/DiagnosticNames.cpp +++ b/clang/tools/diagtool/DiagnosticNames.cpp @@ -23,26 +23,13 @@ llvm::ArrayRef<DiagnosticRecord> diagtool::getBuiltinDiagnosticsByName() { return llvm::ArrayRef(BuiltinDiagnosticsByName); } - // FIXME: Is it worth having two tables, especially when this one can get // out of sync easily? static const DiagnosticRecord BuiltinDiagnosticsByID[] = { #define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR, \ SHOWINSYSHEADER, SHOWINSYSMACRO, DEFER, CATEGORY) \ {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)}, -#include "clang/Basic/DiagnosticCommonKinds.inc" -#include "clang/Basic/DiagnosticCrossTUKinds.inc" -#include "clang/Basic/DiagnosticDriverKinds.inc" -#include "clang/Basic/DiagnosticFrontendKinds.inc" -#include "clang/Basic/DiagnosticSerializationKinds.inc" -#include "clang/Basic/DiagnosticLexKinds.inc" -#include "clang/Basic/DiagnosticParseKinds.inc" -#include "clang/Basic/DiagnosticASTKinds.inc" -#include "clang/Basic/DiagnosticCommentKinds.inc" -#include "clang/Basic/DiagnosticSemaKinds.inc" -#include "clang/Basic/DiagnosticAnalysisKinds.inc" -#include "clang/Basic/DiagnosticRefactoringKinds.inc" -#include "clang/Basic/DiagnosticInstallAPIKinds.inc" +#include "clang/Basic/AllDiagnosticKinds.inc" #undef DIAG }; @@ -54,6 +41,13 @@ static bool orderByID(const DiagnosticRecord &Left, const DiagnosticRecord &diagtool::getDiagnosticForID(short DiagID) { DiagnosticRecord Key = {nullptr, DiagID, 0}; + // The requirement for lower_bound to produce a valid result it is + // enough if the BuiltinDiagnosticsByID is partitioned (by DiagID), + // but as we want this function to work for all possible values of + // DiagID sent in as argument it is better to right away check if + // BuiltinDiagnosticsByID is sorted. + assert(llvm::is_sorted(BuiltinDiagnosticsByID, orderByID) && + "IDs in BuiltinDiagnosticsByID must be sorted."); const DiagnosticRecord *Result = llvm::lower_bound(BuiltinDiagnosticsByID, Key, orderByID); assert(Result && "diagnostic not found; table may be out of date"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
