llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Steven Wu (cachemeifyoucan) <details> <summary>Changes</summary> Sema::UnusedLocalTypedefNameCandidates is populated while iterating a Scope's DeclsInScope, which is a SmallPtrSet whose iteration order depends on pointer values and is therefore not stable across runs. The candidates are serialized into the AST file -- both to assign declaration IDs and to write the UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES record -- and are also used to emit the deferred -Wunused-local-typedef warnings, so neither the emitted PCH/AST file nor the diagnostics were reproducible. With deterministic compilation caching this surfaces as a "cache poisoned" error, because two builds of the same PCH produce different bytes. Sort the candidates by source location at the point they are consumed, via Sema::getSortedUnusedLocalTypedefNameCandidates(), so that both the diagnostics and the serialized declarations are deterministic. --- Full diff: https://github.com/llvm/llvm-project/pull/209639.diff 10 Files Affected: - (modified) clang/include/clang/Sema/ExternalSemaSource.h (+1-1) - (modified) clang/include/clang/Sema/MultiplexExternalSemaSource.h (+1-1) - (modified) clang/include/clang/Sema/Sema.h (+5-2) - (modified) clang/include/clang/Serialization/ASTReader.h (+1-1) - (modified) clang/lib/Sema/MultiplexExternalSemaSource.cpp (+1-1) - (modified) clang/lib/Sema/Sema.cpp (+16-1) - (modified) clang/lib/Serialization/ASTReader.cpp (+1-1) - (modified) clang/lib/Serialization/ASTWriter.cpp (+8-4) - (added) clang/test/PCH/unused-local-typedef-determinism.cpp (+27) - (added) clang/test/SemaCXX/warn-unused-local-typedef-deterministic-order.cpp (+92) ``````````diff diff --git a/clang/include/clang/Sema/ExternalSemaSource.h b/clang/include/clang/Sema/ExternalSemaSource.h index e7e66bea8c6bd..f98b0f3585c03 100644 --- a/clang/include/clang/Sema/ExternalSemaSource.h +++ b/clang/include/clang/Sema/ExternalSemaSource.h @@ -141,7 +141,7 @@ class ExternalSemaSource : public ExternalASTSource { /// be invoked multiple times; the external source should take care not to /// introduce the same declarations repeatedly. virtual void ReadUnusedLocalTypedefNameCandidates( - llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) {} + llvm::SmallPtrSetImpl<const TypedefNameDecl *> &Decls) {} /// Read the set of referenced selectors known to the /// external Sema source. diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h index 1395470401b5d..6c117feaa47e0 100644 --- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h +++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h @@ -301,7 +301,7 @@ class MultiplexExternalSemaSource : public ExternalSemaSource { /// be invoked multiple times; the external source should take care not to /// introduce the same declarations repeatedly. void ReadUnusedLocalTypedefNameCandidates( - llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) override; + llvm::SmallPtrSetImpl<const TypedefNameDecl *> &Decls) override; /// Read the set of referenced selectors known to the /// external Sema source. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index bb5697a16e090..1c343af6008ec 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3609,8 +3609,11 @@ class Sema final : public SemaBase { ExtnameUndeclaredIdentifiers; /// Set containing all typedefs that are likely unused. - llvm::SmallSetVector<const TypedefNameDecl *, 4> - UnusedLocalTypedefNameCandidates; + llvm::SmallPtrSet<const TypedefNameDecl *, 4> UnusedLocalTypedefNameCandidates; + + /// Return UnusedLocalTypedefNameCandidates in a deterministic order. + SmallVector<const TypedefNameDecl *, 4> + getSortedUnusedLocalTypedefNameCandidates() const; typedef LazyVector<const DeclaratorDecl *, ExternalSemaSource, &ExternalSemaSource::ReadUnusedFileScopedDecls, 2, 2> diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 61bd3d0748988..d800af83d350b 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -2317,7 +2317,7 @@ class ASTReader : public ExternalPreprocessorSource, void ReadExtVectorDecls(SmallVectorImpl<TypedefNameDecl *> &Decls) override; void ReadUnusedLocalTypedefNameCandidates( - llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) override; + llvm::SmallPtrSetImpl<const TypedefNameDecl *> &Decls) override; void ReadDeclsToCheckForDeferredDiags( llvm::SmallSetVector<Decl *, 4> &Decls) override; diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp index 4306143025c80..49bd9596f77a7 100644 --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -303,7 +303,7 @@ void MultiplexExternalSemaSource::ReadDeclsToCheckForDeferredDiags( } void MultiplexExternalSemaSource::ReadUnusedLocalTypedefNameCandidates( - llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) { + llvm::SmallPtrSetImpl<const TypedefNameDecl *> &Decls) { for(size_t i = 0; i < Sources.size(); ++i) Sources[i]->ReadUnusedLocalTypedefNameCandidates(Decls); } diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index e689c75eb566c..86e84db546394 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1192,11 +1192,26 @@ static bool IsRecordFullyDefined(const CXXRecordDecl *RD, return Complete; } +SmallVector<const TypedefNameDecl *, 4> +Sema::getSortedUnusedLocalTypedefNameCandidates() const { + // The candidates are collected while iterating a Scope's SmallPtrSet, so sort + // by source location for a deterministic order. + SmallVector<const TypedefNameDecl *, 4> Sorted( + UnusedLocalTypedefNameCandidates.begin(), + UnusedLocalTypedefNameCandidates.end()); + llvm::sort(Sorted, [](const TypedefNameDecl *LHS, const TypedefNameDecl *RHS) { + return LHS->getLocation().getRawEncoding() < + RHS->getLocation().getRawEncoding(); + }); + return Sorted; +} + void Sema::emitAndClearUnusedLocalTypedefWarnings() { if (ExternalSource) ExternalSource->ReadUnusedLocalTypedefNameCandidates( UnusedLocalTypedefNameCandidates); - for (const TypedefNameDecl *TD : UnusedLocalTypedefNameCandidates) { + for (const TypedefNameDecl *TD : + getSortedUnusedLocalTypedefNameCandidates()) { if (TD->isReferenced()) continue; Diag(TD->getLocation(), diag::warn_unused_local_typedef) diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 080cb7df04406..4d3816d686730 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9721,7 +9721,7 @@ void ASTReader::ReadExtVectorDecls(SmallVectorImpl<TypedefNameDecl *> &Decls) { } void ASTReader::ReadUnusedLocalTypedefNameCandidates( - llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) { + llvm::SmallPtrSetImpl<const TypedefNameDecl *> &Decls) { for (unsigned I = 0, N = UnusedLocalTypedefNameCandidates.size(); I != N; ++I) { TypedefNameDecl *D = dyn_cast_or_null<TypedefNameDecl>( diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index b6b576833babb..76a6c99ea5e6f 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5796,8 +5796,10 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) { for (unsigned I = 0, N = SemaRef.VTableUses.size(); I != N; ++I) GetDeclRef(SemaRef.VTableUses[I].first); - // Writing all of the UnusedLocalTypedefNameCandidates. - for (const TypedefNameDecl *TD : SemaRef.UnusedLocalTypedefNameCandidates) + // Writing all of the UnusedLocalTypedefNameCandidates in a deterministic + // order. + for (const TypedefNameDecl *TD : + SemaRef.getSortedUnusedLocalTypedefNameCandidates()) GetDeclRef(TD); // Writing all of pending implicit instantiations. @@ -5924,9 +5926,11 @@ void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) { Stream.EmitRecord(VTABLE_USES, VTableUses); } - // Write the record containing potentially unused local typedefs. + // Write the record containing potentially unused local typedefs, in a + // deterministic order. RecordData UnusedLocalTypedefNameCandidates; - for (const TypedefNameDecl *TD : SemaRef.UnusedLocalTypedefNameCandidates) + for (const TypedefNameDecl *TD : + SemaRef.getSortedUnusedLocalTypedefNameCandidates()) AddEmittedDeclRef(TD, UnusedLocalTypedefNameCandidates); if (!UnusedLocalTypedefNameCandidates.empty()) Stream.EmitRecord(UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES, diff --git a/clang/test/PCH/unused-local-typedef-determinism.cpp b/clang/test/PCH/unused-local-typedef-determinism.cpp new file mode 100644 index 0000000000000..dc4753b9490a8 --- /dev/null +++ b/clang/test/PCH/unused-local-typedef-determinism.cpp @@ -0,0 +1,27 @@ +// Check that emitting a PCH is deterministic even when a scope contains many +// unused local typedefs. These are collected into +// Sema::UnusedLocalTypedefNameCandidates while iterating a Scope's SmallPtrSet +// (a pointer-order, run-to-run unstable container) and are then serialized into +// the AST file, so without a stable order the two PCHs below would differ. +// +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: %clang_cc1 -x c++-header %s -emit-pch -o %t/a.pch +// RUN: %clang_cc1 -x c++-header %s -emit-pch -o %t/b.pch +// RUN: cmp %t/a.pch %t/b.pch + +inline void f() { + typedef int t00; typedef int t01; typedef int t02; typedef int t03; + typedef int t04; typedef int t05; typedef int t06; typedef int t07; + typedef int t08; typedef int t09; typedef int t10; typedef int t11; + typedef int t12; typedef int t13; typedef int t14; typedef int t15; + typedef int t16; typedef int t17; typedef int t18; typedef int t19; + typedef int t20; typedef int t21; typedef int t22; typedef int t23; + typedef int t24; typedef int t25; typedef int t26; typedef int t27; + typedef int t28; typedef int t29; typedef int t30; typedef int t31; + typedef int t32; typedef int t33; typedef int t34; typedef int t35; + typedef int t36; typedef int t37; typedef int t38; typedef int t39; + typedef int t40; typedef int t41; typedef int t42; typedef int t43; + typedef int t44; typedef int t45; typedef int t46; typedef int t47; + typedef int t48; typedef int t49; +} diff --git a/clang/test/SemaCXX/warn-unused-local-typedef-deterministic-order.cpp b/clang/test/SemaCXX/warn-unused-local-typedef-deterministic-order.cpp new file mode 100644 index 0000000000000..ef0eb25bcbd8d --- /dev/null +++ b/clang/test/SemaCXX/warn-unused-local-typedef-deterministic-order.cpp @@ -0,0 +1,92 @@ +// Verify that -Wunused-local-typedef diagnostics are emitted in a deterministic +// (source) order even when a scope contains many unused local typedefs. The +// candidates are collected while iterating a Scope's DeclsInScope, which is a +// SmallPtrSet, so without sorting the order would depend on pointer values and +// vary across runs. +// +// RUN: %clang_cc1 %s -fsyntax-only -Wunused-local-typedef 2>&1 | FileCheck %s + +inline void f() { + // Enough typedefs to exceed the small storage of Scope::DeclSetTy. + typedef int t01; + typedef int t02; + typedef int t03; + typedef int t04; + typedef int t05; + typedef int t06; + typedef int t07; + typedef int t08; + typedef int t09; + typedef int t10; + typedef int t11; + typedef int t12; + typedef int t13; + typedef int t14; + typedef int t15; + typedef int t16; + typedef int t17; + typedef int t18; + typedef int t19; + typedef int t20; + typedef int t21; + typedef int t22; + typedef int t23; + typedef int t24; + typedef int t25; + typedef int t26; + typedef int t27; + typedef int t28; + typedef int t29; + typedef int t30; + typedef int t31; + typedef int t32; + typedef int t33; + typedef int t34; + typedef int t35; + typedef int t36; + typedef int t37; + typedef int t38; + typedef int t39; + typedef int t40; +} + +// CHECK: warning: unused typedef 't01' +// CHECK: warning: unused typedef 't02' +// CHECK: warning: unused typedef 't03' +// CHECK: warning: unused typedef 't04' +// CHECK: warning: unused typedef 't05' +// CHECK: warning: unused typedef 't06' +// CHECK: warning: unused typedef 't07' +// CHECK: warning: unused typedef 't08' +// CHECK: warning: unused typedef 't09' +// CHECK: warning: unused typedef 't10' +// CHECK: warning: unused typedef 't11' +// CHECK: warning: unused typedef 't12' +// CHECK: warning: unused typedef 't13' +// CHECK: warning: unused typedef 't14' +// CHECK: warning: unused typedef 't15' +// CHECK: warning: unused typedef 't16' +// CHECK: warning: unused typedef 't17' +// CHECK: warning: unused typedef 't18' +// CHECK: warning: unused typedef 't19' +// CHECK: warning: unused typedef 't20' +// CHECK: warning: unused typedef 't21' +// CHECK: warning: unused typedef 't22' +// CHECK: warning: unused typedef 't23' +// CHECK: warning: unused typedef 't24' +// CHECK: warning: unused typedef 't25' +// CHECK: warning: unused typedef 't26' +// CHECK: warning: unused typedef 't27' +// CHECK: warning: unused typedef 't28' +// CHECK: warning: unused typedef 't29' +// CHECK: warning: unused typedef 't30' +// CHECK: warning: unused typedef 't31' +// CHECK: warning: unused typedef 't32' +// CHECK: warning: unused typedef 't33' +// CHECK: warning: unused typedef 't34' +// CHECK: warning: unused typedef 't35' +// CHECK: warning: unused typedef 't36' +// CHECK: warning: unused typedef 't37' +// CHECK: warning: unused typedef 't38' +// CHECK: warning: unused typedef 't39' +// CHECK: warning: unused typedef 't40' `````````` </details> https://github.com/llvm/llvm-project/pull/209639 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
