Skip to content

Conversation

kazutakahirata
Copy link
Contributor

This patch deprecates the redirection from SmallSet to SmallPtrSet.

I attempted to deprecate in the usual manner:

template <typename PointeeType, unsigned N>
LLVM_DEPRECATED("...", "...")
class SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N> {};

However, the deprecation warning wouldn't fire.

For this reason, I've attached a deprecation message to the default
constructor.

This patch deprecates the redirection from SmallSet to SmallPtrSet.

I attempted to deprecate in the usual manner:

template <typename PointeeType, unsigned N>
LLVM_DEPRECATED("...", "...")
class SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N> {};

However, the deprecation warning wouldn't fire.

For this reason, I've attached a deprecation message to the default
constructor.
@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch deprecates the redirection from SmallSet to SmallPtrSet.

I attempted to deprecate in the usual manner:

template <typename PointeeType, unsigned N>
LLVM_DEPRECATED("...", "...")
class SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N> {};

However, the deprecation warning wouldn't fire.

For this reason, I've attached a deprecation message to the default
constructor.


Full diff: https://github.com/llvm/llvm-project/pull/154891.diff

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/SmallSet.h (+7-1)
diff --git a/llvm/include/llvm/ADT/SmallSet.h b/llvm/include/llvm/ADT/SmallSet.h
index eb434bcb71717..427a89c906fd3 100644
--- a/llvm/include/llvm/ADT/SmallSet.h
+++ b/llvm/include/llvm/ADT/SmallSet.h
@@ -269,7 +269,13 @@ class SmallSet {
 /// If this set is of pointer values, transparently switch over to using
 /// SmallPtrSet for performance.
 template <typename PointeeType, unsigned N>
-class SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N> {};
+class SmallSet<PointeeType *, N> : public SmallPtrSet<PointeeType *, N> {
+public:
+  // LLVM_DEPRECATED placed between "template" and "class" above won't work for
+  // some reason.  Put a deprecation message on the default constructor instead.
+  LLVM_DEPRECATED("Use SmallPtrSet instead", "SmallPtrSet")
+  SmallSet<PointeeType *, N>() = default;
+};
 
 /// Equality comparison for SmallSet.
 ///

@zwuis
Copy link
Contributor

zwuis commented Aug 22, 2025

[[deprecated]] struct S1 {} var1; // `var1` is deprecated
struct [[deprecated]] S2 {} var2; // `S2` is deprecated

Copy link

github-actions bot commented Aug 22, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@zwuis
Copy link
Contributor

zwuis commented Aug 23, 2025

[[deprecated]] struct S1 {} var1; // `var1` is deprecated
struct [[deprecated]] S2 {} var2; // `S2` is deprecated

Did you try

template <typename PointeeType, unsigned N>
class LLVM_DEPRECATED("...", "...") SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N> {};

@kazutakahirata
Copy link
Contributor Author

Did you try

template <typename PointeeType, unsigned N>
class LLVM_DEPRECATED("...", "...") SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N> {};

@zwuis Yes, I just tried. I still don't get a warning when I intentionally place SmallSet<int *, 4> Set; in one of the .cpp files. I think I have to go with the constructor approach. Thank for the suggestion though!

@kazutakahirata kazutakahirata merged commit 01bbbb5 into llvm:main Aug 23, 2025
9 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250821_deprecate_SmallSet_ptr branch August 23, 2025 03:38
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 23, 2025

LLVM Buildbot has detected a new failure on builder clang-m68k-linux-cross running on suse-gary-m68k-cross while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/15019

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
...
[128/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/MacroExpansionContextTest.cpp.o
In file included from /var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Parse/Parser.h:20,
                 from /var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/MacroExpansionContextTest.cpp:23:
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:850:7: warning: ‘clang::Sema’ declared with greater visibility than the type of its field ‘clang::Sema::UnusedFileScopedDecls’ [-Wattributes]
  850 | class Sema final : public SemaBase {
      |       ^~~~
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:850:7: warning: ‘clang::Sema’ declared with greater visibility than the type of its field ‘clang::Sema::TentativeDefinitions’ [-Wattributes]
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:850:7: warning: ‘clang::Sema’ declared with greater visibility than the type of its field ‘clang::Sema::ExtVectorDecls’ [-Wattributes]
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:850:7: warning: ‘clang::Sema’ declared with greater visibility than the type of its field ‘clang::Sema::DelegatingCtorDecls’ [-Wattributes]
[129/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp.o
FAILED: tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp.o 
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/llvm/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Tooling -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googletest/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googlemock/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -O3 -DNDEBUG -std=c++17  -Wno-variadic-macros -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp.o -MF tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp.o.d -o tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp.o -c /var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
c++: fatal error: Killed signal terminated program cc1plus
compilation terminated.
[130/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MapLatticeTest.cpp.o
[131/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp.o
[132/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/APSIntTypeTest.cpp.o
[133/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/RecordOpsTest.cpp.o
[134/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/CFGDominatorTree.cpp.o
[135/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp.o
[136/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/CallEventTest.cpp.o
[137/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DebugSupportTest.cpp.o
[138/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/ASTOpsTest.cpp.o
[139/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp.o
[140/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/WatchedLiteralsSolverTest.cpp.o
[141/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp.o
[142/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/LifetimeSafetyTest.cpp.o
[143/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp.o
[144/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/CFGTest.cpp.o
[145/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MatchSwitchTest.cpp.o
[146/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/BlockEntranceCallbackTest.cpp.o
[147/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TransferBranchTest.cpp.o
[148/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SmartPointerAccessorCachingTest.cpp.o
[149/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DeterminismTest.cpp.o
[150/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/ConflictingEvalCallsTest.cpp.o
[151/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SimplifyConstraintsTest.cpp.o
[152/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/LoggerTest.cpp.o
[153/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o
[154/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/BugReportInterestingnessTest.cpp.o
[155/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TestingSupportTest.cpp.o
[156/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SignAnalysisTest.cpp.o
[157/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp.o
[158/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/ExprMutationAnalyzerTest.cpp.o
[159/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/CallDescriptionTest.cpp.o
[160/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TestingSupport.cpp.o
[161/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp.o
[162/430] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TransferTest.cpp.o
ninja: build stopped: subcommand failed.

rupprecht added a commit that referenced this pull request Aug 23, 2025
…#155075)

Reverts #154891

The added constructors changes the semantics of other implicitly defined
constructors, which causes downstream breakages.

```
... error: object of type 'X' cannot be assigned because its copy assignment operator is implicitly deleted
... note: explicitly defaulted function was implicitly deleted here
... note: copy assignment operator of 'X' is implicitly deleted because field 'x' has a deleted copy assignment operator
llvm/include/llvm/ADT/SmallSet.h:283:3: note: copy assignment operator is implicitly deleted because 'SmallSet<const XX *, 2>' has a user-declared move constructor
  283 |   SmallSet(SmallSet &&) = default;
```
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Aug 23, 2025
…mallPtrSet" (#155075)

Reverts llvm/llvm-project#154891

The added constructors changes the semantics of other implicitly defined
constructors, which causes downstream breakages.

```
... error: object of type 'X' cannot be assigned because its copy assignment operator is implicitly deleted
... note: explicitly defaulted function was implicitly deleted here
... note: copy assignment operator of 'X' is implicitly deleted because field 'x' has a deleted copy assignment operator
llvm/include/llvm/ADT/SmallSet.h:283:3: note: copy assignment operator is implicitly deleted because 'SmallSet<const XX *, 2>' has a user-declared move constructor
  283 |   SmallSet(SmallSet &&) = default;
```
kazutakahirata added a commit to kazutakahirata/llvm-project that referenced this pull request Aug 23, 2025
This patch deprecates the SmallSet specialization for pointer types,
which redirects to SmallPtrSet.

My previous attempt in llvm#154891 broke downstream users.  Adding
user-defined constructors with LLVM_DEPRECATED inadvertently caused
the compiler to delete the copy and move assignment operators.

This iteration sidesteps the "Rule of Five" issue entirely by
introducing an intermediate class, DeprecatedSmallSet.  The
deprecation attribute is attached to this new class, and SmallSet
specialization inherits from it.
kazutakahirata added a commit that referenced this pull request Aug 23, 2025
…#155078)

This patch deprecates the SmallSet specialization for pointer types,
which redirects to SmallPtrSet.

My previous attempt in #154891 broke downstream users.  Adding
user-defined constructors with LLVM_DEPRECATED inadvertently caused
the compiler to delete the copy and move assignment operators.

This iteration sidesteps the "Rule of Five" issue entirely by
introducing an intermediate class, DeprecatedSmallSet.  The
deprecation attribute is attached to this new class, and SmallSet
specialization inherits from it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants