[PATCH] D90555: [clangd] Handle duplicate enum constants in PopulateSwitch tweak

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
njames93 requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

If an enum has different names for the same constant, make sure only the first 
one declared gets added into the switch. Failing to do so results in a compiler 
error as 2 case labels can't represent the same value.

  enum Numbers{
  One,
  Un = One,
  Two,
  Deux = Two,
  Three,
  Trois = Three
  };
  
  // Old behaviour
  switch (<Number>) {
    case One:
    case Un:
    case Two:
    case Duex:
    case Three:
    case Trois: break;
  }
  
  // New behaviour
  switch (<Number>) {
    case One:
    case Two:
    case Three: break;
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90555

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2980,6 +2980,12 @@
             void function() { switch (ns::A) {case ns::A:break;} }
           )"",
       },
+      {
+          // Selection on duplicate token enum
+          Function,
+          R""(enum Enum {A,B,b=B}; ^switch (A) {})"",
+          R""(enum Enum {A,B,b=B}; switch (A) {case A:case B:break;})"",
+      },
   };
 
   for (const auto &Case : Cases) {
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -166,7 +166,10 @@
 
   std::string Text;
   for (EnumConstantDecl *Enumerator : EnumD->enumerators()) {
-    if (ExistingEnumerators.contains(Enumerator->getInitVal()))
+    // Try to insert this Enumerator into the set. If this fails, the value was
+    // either already there to begin with or we have already added it using a
+    // different name.
+    if (!ExistingEnumerators.insert(Enumerator->getInitVal()).second)
       continue;
 
     Text += "case ";



Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2980,6 +2980,12 @@
             void function() { switch (ns::A) {case ns::A:break;} }
           )"",
       },
+      {
+          // Selection on duplicate token enum
+          Function,
+          R""(enum Enum {A,B,b=B}; ^switch (A) {})"",
+          R""(enum Enum {A,B,b=B}; switch (A) {case A:case B:break;})"",
+      },
   };
 
   for (const auto &Case : Cases) {
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -166,7 +166,10 @@
 
   std::string Text;
   for (EnumConstantDecl *Enumerator : EnumD->enumerators()) {
-    if (ExistingEnumerators.contains(Enumerator->getInitVal()))
+    // Try to insert this Enumerator into the set. If this fails, the value was
+    // either already there to begin with or we have already added it using a
+    // different name.
+    if (!ExistingEnumerators.insert(Enumerator->getInitVal()).second)
       continue;
 
     Text += "case ";

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits