benlangmuir created this revision.
benlangmuir added reviewers: steven_wu, jansvoboda11, akyrtzi.
Herald added a subscriber: mgrang.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When writing a pcm, we serialize diagnostic mappings in order to
accurately reproduce the diagnostic environment inside any headers from
that module. However, the diagnostic state mapping table contains
entries for every diagnostic ID ever accessed, while we only want to
serialize the ones that are actually modified from their default value.
Futher, we need to serialize them in a deterministic order.
rdar://111477511
Repository:
rG LLVM Github Monorepo
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,52 @@
+// 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
+// Number of mappings = 3
+// CHECK-SAME: op2=3
+// Common diag id is < 1000 (see DiagnosticIDs.h)
+// CHECK-SAME: op3={{[0-9][0-9]?[0-9]?}} op4=
+// Parse diag id is somewhere in 1000..2999, leaving room for changes
+// CHECK-SAME: op5={{[12][0-9][0-9][0-9]}} op6=
+// Sema diag id is > 2000
+// CHECK-SAME: op7={{[2-9][0-9][0-9][0-9]}} op8=
+
+// 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
+ ;
+// Sema warning
+ int x;
+}
+
+//--- 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
