Skip to content

Commit 7347870

Browse files
karka228karka228
authored
[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.
1 parent 58903c9 commit 7347870

File tree

3 files changed

+44
-59
lines changed

3 files changed

+44
-59
lines changed

‎clang/include/clang/Basic/AllDiagnosticKinds.inc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//===--- AllDiagnosticKinds.inc----------------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
///
9+
/// \file
10+
/// Defines the Diagnostic IDs in ID sorted order. The order is dictated by
11+
/// the enum in DiagnosticIDs.h#L49-L65.
12+
///
13+
//===----------------------------------------------------------------------===//
14+
15+
// Turn off clang-format, as the order of the includes are important to make
16+
// sure tables based on Diagnostic IDs are partitioned/sorted based on
17+
// DiagID.
18+
19+
// clang-format off
20+
#include "clang/Basic/DiagnosticCommonKinds.inc"
21+
#include "clang/Basic/DiagnosticDriverKinds.inc"
22+
#include "clang/Basic/DiagnosticFrontendKinds.inc"
23+
#include "clang/Basic/DiagnosticSerializationKinds.inc"
24+
#include "clang/Basic/DiagnosticLexKinds.inc"
25+
#include "clang/Basic/DiagnosticParseKinds.inc"
26+
#include "clang/Basic/DiagnosticASTKinds.inc"
27+
#include "clang/Basic/DiagnosticCommentKinds.inc"
28+
#include "clang/Basic/DiagnosticCrossTUKinds.inc"
29+
#include "clang/Basic/DiagnosticSemaKinds.inc"
30+
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
31+
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
32+
#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
33+
// clang-format on

‎clang/lib/Basic/DiagnosticIDs.cpp

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -37,43 +37,15 @@ struct StaticDiagInfoDescriptionStringTable {
3737
#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
3838
SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY) \
3939
char ENUM##_desc[sizeof(DESC)];
40-
// clang-format off
41-
#include "clang/Basic/DiagnosticCommonKinds.inc"
42-
#include "clang/Basic/DiagnosticDriverKinds.inc"
43-
#include "clang/Basic/DiagnosticFrontendKinds.inc"
44-
#include "clang/Basic/DiagnosticSerializationKinds.inc"
45-
#include "clang/Basic/DiagnosticLexKinds.inc"
46-
#include "clang/Basic/DiagnosticParseKinds.inc"
47-
#include "clang/Basic/DiagnosticASTKinds.inc"
48-
#include "clang/Basic/DiagnosticCommentKinds.inc"
49-
#include "clang/Basic/DiagnosticCrossTUKinds.inc"
50-
#include "clang/Basic/DiagnosticSemaKinds.inc"
51-
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
52-
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
53-
#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
54-
// clang-format on
40+
#include "clang/Basic/AllDiagnosticKinds.inc"
5541
#undef DIAG
5642
};
5743

5844
const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = {
5945
#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
6046
SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY) \
6147
DESC,
62-
// clang-format off
63-
#include "clang/Basic/DiagnosticCommonKinds.inc"
64-
#include "clang/Basic/DiagnosticDriverKinds.inc"
65-
#include "clang/Basic/DiagnosticFrontendKinds.inc"
66-
#include "clang/Basic/DiagnosticSerializationKinds.inc"
67-
#include "clang/Basic/DiagnosticLexKinds.inc"
68-
#include "clang/Basic/DiagnosticParseKinds.inc"
69-
#include "clang/Basic/DiagnosticASTKinds.inc"
70-
#include "clang/Basic/DiagnosticCommentKinds.inc"
71-
#include "clang/Basic/DiagnosticCrossTUKinds.inc"
72-
#include "clang/Basic/DiagnosticSemaKinds.inc"
73-
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
74-
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
75-
#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
76-
// clang-format on
48+
#include "clang/Basic/AllDiagnosticKinds.inc"
7749
#undef DIAG
7850
};
7951

@@ -85,21 +57,7 @@ const uint32_t StaticDiagInfoDescriptionOffsets[] = {
8557
#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
8658
SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY) \
8759
offsetof(StaticDiagInfoDescriptionStringTable, ENUM##_desc),
88-
// clang-format off
89-
#include "clang/Basic/DiagnosticCommonKinds.inc"
90-
#include "clang/Basic/DiagnosticDriverKinds.inc"
91-
#include "clang/Basic/DiagnosticFrontendKinds.inc"
92-
#include "clang/Basic/DiagnosticSerializationKinds.inc"
93-
#include "clang/Basic/DiagnosticLexKinds.inc"
94-
#include "clang/Basic/DiagnosticParseKinds.inc"
95-
#include "clang/Basic/DiagnosticASTKinds.inc"
96-
#include "clang/Basic/DiagnosticCommentKinds.inc"
97-
#include "clang/Basic/DiagnosticCrossTUKinds.inc"
98-
#include "clang/Basic/DiagnosticSemaKinds.inc"
99-
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
100-
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
101-
#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
102-
// clang-format on
60+
#include "clang/Basic/AllDiagnosticKinds.inc"
10361
#undef DIAG
10462
};
10563

‎clang/tools/diagtool/DiagnosticNames.cpp

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,13 @@ llvm::ArrayRef<DiagnosticRecord> diagtool::getBuiltinDiagnosticsByName() {
2323
return llvm::ArrayRef(BuiltinDiagnosticsByName);
2424
}
2525

26-
2726
// FIXME: Is it worth having two tables, especially when this one can get
2827
// out of sync easily?
2928
static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
3029
#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR, \
3130
SHOWINSYSHEADER, SHOWINSYSMACRO, DEFER, CATEGORY) \
3231
{#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)},
33-
#include "clang/Basic/DiagnosticCommonKinds.inc"
34-
#include "clang/Basic/DiagnosticCrossTUKinds.inc"
35-
#include "clang/Basic/DiagnosticDriverKinds.inc"
36-
#include "clang/Basic/DiagnosticFrontendKinds.inc"
37-
#include "clang/Basic/DiagnosticSerializationKinds.inc"
38-
#include "clang/Basic/DiagnosticLexKinds.inc"
39-
#include "clang/Basic/DiagnosticParseKinds.inc"
40-
#include "clang/Basic/DiagnosticASTKinds.inc"
41-
#include "clang/Basic/DiagnosticCommentKinds.inc"
42-
#include "clang/Basic/DiagnosticSemaKinds.inc"
43-
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
44-
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
45-
#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
32+
#include "clang/Basic/AllDiagnosticKinds.inc"
4633
#undef DIAG
4734
};
4835

@@ -54,6 +41,13 @@ static bool orderByID(const DiagnosticRecord &Left,
5441
const DiagnosticRecord &diagtool::getDiagnosticForID(short DiagID) {
5542
DiagnosticRecord Key = {nullptr, DiagID, 0};
5643

44+
// The requirement for lower_bound to produce a valid result it is
45+
// enough if the BuiltinDiagnosticsByID is partitioned (by DiagID),
46+
// but as we want this function to work for all possible values of
47+
// DiagID sent in as argument it is better to right away check if
48+
// BuiltinDiagnosticsByID is sorted.
49+
assert(llvm::is_sorted(BuiltinDiagnosticsByID, orderByID) &&
50+
"IDs in BuiltinDiagnosticsByID must be sorted.");
5751
const DiagnosticRecord *Result =
5852
llvm::lower_bound(BuiltinDiagnosticsByID, Key, orderByID);
5953
assert(Result && "diagnostic not found; table may be out of date");

0 commit comments

Comments
 (0)