benlangmuir updated this revision to Diff 535882.
benlangmuir added a comment.
Extended test to include diagnostic pragmas
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154016/new/
https://reviews.llvm.org/D154016
Files:
clang/include/clang/Basic/DiagnosticIDs.h
clang/lib/Basic/Diagnostic.cpp
clang/lib/Basic/DiagnosticIDs.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/diag-mappings.c
Index: clang/test/Modules/diag-mappings.c
===================================================================
--- /dev/null
+++ clang/test/Modules/diag-mappings.c
@@ -0,0 +1,94 @@
+// Test that diagnostic mappings are emitted only when needed and in order of
+// diagnostic ID rather than non-deterministically. This test passes 3
+// -W options and expects exactly 3 mappings to be emitted in the pcm. The -W
+// options are chosen to be far apart in ID (see DiagnosticIDs.h) so we can
+// check they are ordered. We also intentionally trigger several other warnings
+// inside the module and ensure they do not show up in the pcm as mappings.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
+// RUN: -fmodules-cache-path=%t/cache -triple x86_64-apple-macosx10.11.0 \
+// RUN: %t/main.m -fdisable-module-hash \
+// RUN: -Werror=stack-protector -Werror=empty-translation-unit -Werror=float-equal
+
+// RUN: mv %t/cache/A.pcm %t/A1.pcm
+
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/A1.pcm | FileCheck %s
+
+// CHECK: <DIAG_PRAGMA_MAPPINGS
+
+// == Initial mappings
+// Number of mappings = 3
+// CHECK-SAME: op2=3
+// Common diag id is < 1000 (see DiagnosticIDs.h)
+// CHECK-SAME: op3=[[STACK_PROT:[0-9][0-9]?[0-9]?]] op4=
+// Parse diag id is somewhere in 1000..2999, leaving room for changes
+// CHECK-SAME: op5=[[EMPTY_TU:[12][0-9][0-9][0-9]]] op6=
+// Sema diag id is > 2000
+// CHECK-SAME: op7=[[FLOAT_EQ:[2-9][0-9][0-9][0-9]]] op8=
+
+// == Pragmas:
+// Each pragma creates a mapping table; and each copies the previous table. The
+// initial mappings are copied as well, but are not serialized since they have
+// isPragma=false.
+
+// == ignored "-Wfloat-equal"
+// CHECK-SAME: op{{[0-9]+}}=1
+// CHECK-SAME: op{{[0-9]+}}=[[FLOAT_EQ]] op{{[0-9]+}}=
+
+// == ignored "-Wstack-protector"
+// CHECK-SAME: op{{[0-9]+}}=2
+// CHECK-SAME: op{{[0-9]+}}=[[STACK_PROT]] op{{[0-9]+}}=
+// CHECK-SAME: op{{[0-9]+}}=[[FLOAT_EQ]] op{{[0-9]+}}=
+
+// == warning "-Wempty-translation-unit"
+// CHECK-SAME: op{{[0-9]+}}=3
+// CHECK-SAME: op{{[0-9]+}}=[[STACK_PROT]] op{{[0-9]+}}=
+// CHECK-SAME: op{{[0-9]+}}=[[EMPTY_TU]] op{{[0-9]+}}=
+// CHECK-SAME: op{{[0-9]+}}=[[FLOAT_EQ]] op{{[0-9]+}}=
+
+// == warning "-Wstack-protector"
+// CHECK-SAME: op{{[0-9]+}}=3
+// CHECK-SAME: op{{[0-9]+}}=[[STACK_PROT]] op{{[0-9]+}}=
+// CHECK-SAME: op{{[0-9]+}}=[[EMPTY_TU]] op{{[0-9]+}}=
+// CHECK-SAME: op{{[0-9]+}}=[[FLOAT_EQ]] op{{[0-9]+}}=
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
+// RUN: -fmodules-cache-path=%t/cache -triple x86_64-apple-macosx10.11.0 \
+// RUN: %t/main.m -fdisable-module-hash \
+// RUN: -Werror=stack-protector -Werror=empty-translation-unit -Werror=float-equal
+
+// RUN: diff %t/cache/A.pcm %t/A1.pcm
+
+//--- module.modulemap
+module A { header "a.h" }
+
+//--- a.h
+// Lex warning
+#warning "w"
+
+static inline void f() {
+// Parse warning
+ ;
+}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wfloat-equal"
+#pragma clang diagnostic ignored "-Wstack-protector"
+
+static inline void g() {
+// Sema warning
+ int x;
+}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wempty-translation-unit"
+#pragma clang diagnostic warning "-Wstack-protector"
+
+#pragma clang diagnostic pop
+#pragma clang diagnostic pop
+
+//--- main.m
+#import "a.h"
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2997,20 +2997,41 @@
assert(Flags == EncodeDiagStateFlags(State) &&
"diag state flags vary in single AST file");
+ // If we ever serialize non-pragma mappings outside the initial state, the
+ // code below will need to consider more than getDefaultMapping.
+ assert(!IncludeNonPragmaStates ||
+ State == Diag.DiagStatesByLoc.FirstDiagState);
+
unsigned &DiagStateID = DiagStateIDMap[State];
Record.push_back(DiagStateID);
if (DiagStateID == 0) {
DiagStateID = ++CurrID;
+ SmallVector<std::pair<unsigned, DiagnosticMapping>> Mappings;
// Add a placeholder for the number of mappings.
auto SizeIdx = Record.size();
Record.emplace_back();
for (const auto &I : *State) {
- if (I.second.isPragma() || IncludeNonPragmaStates) {
- Record.push_back(I.first);
- Record.push_back(I.second.serialize());
- }
+ // Maybe skip non-pragmas.
+ if (!I.second.isPragma() && !IncludeNonPragmaStates)
+ continue;
+ // Skip default mappings. We have a mapping for every diagnostic ever
+ // emitted, regardless of whether it was customized.
+ if (!I.second.isPragma() &&
+ I.second == DiagnosticIDs::getDefaultMapping(I.first))
+ continue;
+ Mappings.push_back(I);
+ }
+
+ // Sort by diag::kind for deterministic output.
+ llvm::sort(Mappings, [](const auto &LHS, const auto &RHS) {
+ return LHS.first < RHS.first;
+ });
+
+ for (const auto &I : Mappings) {
+ Record.push_back(I.first);
+ Record.push_back(I.second.serialize());
}
// Update the placeholder.
Record[SizeIdx] = (Record.size() - SizeIdx) / 2;
Index: clang/lib/Basic/DiagnosticIDs.cpp
===================================================================
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -256,7 +256,7 @@
return Found;
}
-static DiagnosticMapping GetDefaultDiagMapping(unsigned DiagID) {
+DiagnosticMapping DiagnosticIDs::getDefaultMapping(unsigned DiagID) {
DiagnosticMapping Info = DiagnosticMapping::Make(
diag::Severity::Fatal, /*IsUser=*/false, /*IsPragma=*/false);
@@ -293,21 +293,6 @@
};
}
-// Unfortunately, the split between DiagnosticIDs and Diagnostic is not
-// particularly clean, but for now we just implement this method here so we can
-// access GetDefaultDiagMapping.
-DiagnosticMapping &
-DiagnosticsEngine::DiagState::getOrAddMapping(diag::kind Diag) {
- std::pair<iterator, bool> Result =
- DiagMap.insert(std::make_pair(Diag, DiagnosticMapping()));
-
- // Initialize the entry if we added it.
- if (Result.second)
- Result.first->second = GetDefaultDiagMapping(Diag);
-
- return Result.first->second;
-}
-
static const StaticDiagCategoryRec CategoryNameTable[] = {
#define GET_CATEGORY_TABLE
#define CATEGORY(X, ENUM) { X, STR_SIZE(X, uint8_t) },
@@ -449,7 +434,7 @@
return false;
EnabledByDefault =
- GetDefaultDiagMapping(DiagID).getSeverity() != diag::Severity::Ignored;
+ getDefaultMapping(DiagID).getSeverity() != diag::Severity::Ignored;
return true;
}
@@ -457,7 +442,7 @@
if (DiagID >= diag::DIAG_UPPER_LIMIT)
return false;
- return GetDefaultDiagMapping(DiagID).getSeverity() >= diag::Severity::Error;
+ return getDefaultMapping(DiagID).getSeverity() >= diag::Severity::Error;
}
/// getDescription - Given a diagnostic ID, return a description of the
Index: clang/lib/Basic/Diagnostic.cpp
===================================================================
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -160,6 +160,18 @@
Report(ID) << DelayedDiagArg1 << DelayedDiagArg2 << DelayedDiagArg3;
}
+DiagnosticMapping &
+DiagnosticsEngine::DiagState::getOrAddMapping(diag::kind Diag) {
+ std::pair<iterator, bool> Result =
+ DiagMap.insert(std::make_pair(Diag, DiagnosticMapping()));
+
+ // Initialize the entry if we added it.
+ if (Result.second)
+ Result.first->second = DiagnosticIDs::getDefaultMapping(Diag);
+
+ return Result.first->second;
+}
+
void DiagnosticsEngine::DiagStateMap::appendFirst(DiagState *State) {
assert(Files.empty() && "not first");
FirstDiagState = CurDiagState = State;
Index: clang/include/clang/Basic/DiagnosticIDs.h
===================================================================
--- clang/include/clang/Basic/DiagnosticIDs.h
+++ clang/include/clang/Basic/DiagnosticIDs.h
@@ -159,6 +159,10 @@
Result.Severity = Bits & 0x7;
return Result;
}
+
+ bool operator==(DiagnosticMapping Other) const {
+ return serialize() == Other.serialize();
+ }
};
/// Used for handling and querying diagnostic IDs.
@@ -208,6 +212,9 @@
/// default.
static bool isDefaultMappingAsError(unsigned DiagID);
+ /// Get the default mapping for this diagnostic.
+ static DiagnosticMapping getDefaultMapping(unsigned DiagID);
+
/// Determine whether the given built-in diagnostic ID is a Note.
static bool isBuiltinNote(unsigned DiagID);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
