[clang] 7347870 - [diagtool] Make the BuiltinDiagnosticsByID table sorted (#120321)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 17 01:23:31 PST 2025
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");
More information about the cfe-commits
mailing list