llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Michael Park (mpark) <details> <summary>Changes</summary> In https://reviews.llvm.org/D137214 and https://reviews.llvm.org/D136624, offset adjustment logic was added to account for the non-affecting module map files that are removed. While the adjustment logic applies to global source location offsets, they do not apply to file-internal offsets (relative within the file). In `ASTWriter::WritePragmaDiagnosticMappings`, the adjustment is applied to `StatePoint.Offset`s in `StateTransitions`. However, these offsets are file-internal offsets, not global source location offsets. As such, applying adjustment to these offsets result in incorrect diagnostic behavior from the module. Specifically, wrapping a piece of code in `pragma clang diagnostic push/pop`, inside of a module is not applied correctly. A new test case `diag-pragma-nonaffecting.cpp` was added to verify the broken behavior as well as the corrected behavior with this commit. Assisted-by: Claude Code --- Full diff: https://github.com/llvm/llvm-project/pull/184956.diff 2 Files Affected: - (modified) clang/lib/Serialization/ASTWriter.cpp (+1-1) - (added) clang/test/Modules/diag-pragma-nonaffecting.cpp (+50) ``````````diff diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 814f4e42e9c9b..ec718169550aa 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3357,7 +3357,7 @@ void ASTWriter::WritePragmaDiagnosticMappings(const DiagnosticsEngine &Diag, Record.push_back(FileIDAndFile.second.StateTransitions.size()); for (auto &StatePoint : FileIDAndFile.second.StateTransitions) { - Record.push_back(getAdjustedOffset(StatePoint.Offset)); + Record.push_back(StatePoint.Offset); AddDiagState(StatePoint.State, false); } } diff --git a/clang/test/Modules/diag-pragma-nonaffecting.cpp b/clang/test/Modules/diag-pragma-nonaffecting.cpp new file mode 100644 index 0000000000000..3bbeefc320b9b --- /dev/null +++ b/clang/test/Modules/diag-pragma-nonaffecting.cpp @@ -0,0 +1,50 @@ +// Test that pragma diagnostic mappings from an explicit module are not +// corrupted by the presence of non-affecting module map files. +// +// When non-affecting module map files are pruned, NonAffectingRanges becomes +// non-empty. Previously, getAdjustedOffset was incorrectly applied to +// file-internal byte offsets in WritePragmaDiagnosticMappings, corrupting the +// serialized diagnostic state transition offsets. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// Build the module with a non-affecting module map present. +// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps \ +// RUN: -fmodule-map-file=%t/nonaffecting/module.modulemap \ +// RUN: -emit-module -fmodule-name=diag_pragma_mod \ +// RUN: -x c++ %t/module.modulemap -o %t/diag_pragma_mod.pcm + +// Use the module and verify the warning is suppressed. +// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps \ +// RUN: -fmodule-file=%t/diag_pragma_mod.pcm \ +// RUN: -I %t -verify %t/tu.cpp + +//--- module.modulemap +module diag_pragma_mod { header "header.h" } + +//--- header.h +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wstring-plus-int" +template<typename T> const char *suppressed(T t) { + return "foo" + t; +} +#pragma clang diagnostic pop + +template<typename T> const char *unsuppressed(T t) { + return "bar" + t; +} + +//--- nonaffecting/module.modulemap +module nonaffecting {} + +//--- tu.cpp +#include "header.h" + +void test() { + suppressed(0); // no warning expected - suppressed by pragma in module + + // [email protected]:9 {{adding 'int' to a string}} + // [email protected]:9 {{use array indexing}} + unsuppressed(0); // expected-note {{in instantiation of}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/184956 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
