[clang] [diagtool] Make the BuiltinDiagnosticsByID table sorted (PR #120321)
Karl-Johan Karlsson via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 7 07:20:26 PST 2025
https://github.com/karka228 updated https://github.com/llvm/llvm-project/pull/120321
>From 1ad62a9a136a5ae80c880459472a88517afe75a4 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Tue, 17 Dec 2024 22:48:04 +0100
Subject: [PATCH 1/2] [diagtool] Make the BuiltinDiagnosticsByID table sorted
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.
---
clang/tools/diagtool/DiagnosticNames.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/tools/diagtool/DiagnosticNames.cpp b/clang/tools/diagtool/DiagnosticNames.cpp
index eb90f082437b33..215087b9004959 100644
--- a/clang/tools/diagtool/DiagnosticNames.cpp
+++ b/clang/tools/diagtool/DiagnosticNames.cpp
@@ -23,15 +23,14 @@ 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?
+// clang-format off
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"
@@ -39,12 +38,14 @@ static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
#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"
#undef DIAG
};
+// clang-format on
static bool orderByID(const DiagnosticRecord &Left,
const DiagnosticRecord &Right) {
>From 4cedc90e9b26e5d48d53df463e8650d8a726e1d5 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Tue, 7 Jan 2025 15:59:16 +0100
Subject: [PATCH 2/2] [diagtool] Make the BuiltinDiagnosticsByID table sorted
Extracted the includes into a wrapper header file DiagnosticIDs.inc.
Added is_sorted assert.
---
clang/include/clang/Basic/DiagnosticIDs.inc | 31 +++++++++++++
clang/lib/Basic/DiagnosticIDs.cpp | 48 ++-------------------
clang/tools/diagtool/DiagnosticNames.cpp | 23 ++++------
3 files changed, 42 insertions(+), 60 deletions(-)
create mode 100644 clang/include/clang/Basic/DiagnosticIDs.inc
diff --git a/clang/include/clang/Basic/DiagnosticIDs.inc b/clang/include/clang/Basic/DiagnosticIDs.inc
new file mode 100644
index 00000000000000..69a7c03d39dff0
--- /dev/null
+++ b/clang/include/clang/Basic/DiagnosticIDs.inc
@@ -0,0 +1,31 @@
+//===--- DiagnosticIDs.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.
+///
+//===----------------------------------------------------------------------===//
+
+// Turn off clang-format, as the order of the includes are important to make
+// sure the table is sorted.
+
+// 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..3c82bbb48c4c4b 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/DiagnosticIDs.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/DiagnosticIDs.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/DiagnosticIDs.inc"
#undef DIAG
};
diff --git a/clang/tools/diagtool/DiagnosticNames.cpp b/clang/tools/diagtool/DiagnosticNames.cpp
index 215087b9004959..b89e5f0397a84f 100644
--- a/clang/tools/diagtool/DiagnosticNames.cpp
+++ b/clang/tools/diagtool/DiagnosticNames.cpp
@@ -25,27 +25,13 @@ llvm::ArrayRef<DiagnosticRecord> diagtool::getBuiltinDiagnosticsByName() {
// FIXME: Is it worth having two tables, especially when this one can get
// out of sync easily?
-// clang-format off
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/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"
+#include "clang/Basic/DiagnosticIDs.inc"
#undef DIAG
};
-// clang-format on
static bool orderByID(const DiagnosticRecord &Left,
const DiagnosticRecord &Right) {
@@ -55,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