[PATCH] Fix an invalid check for duplicate option categories.

[Alexander Kornienko]

Hi jordan_rose,

The check performed in the comparator is invalid, as some STL
implementations enforce strict weak ordering by calling the comparator with the
same value. This check was also in a wrong place: the assertion would only fire
when -help was used. The new check is performed each time the category is
registered (we are not going to have thousands of them, so it's fine to do it in
O(N^2)).

http://llvm-reviews.chandlerc.com/D2699

Files:
  include/llvm/Support/CommandLine.h
  lib/Support/CommandLine.cpp

Index: include/llvm/Support/CommandLine.h
===================================================================
--- include/llvm/Support/CommandLine.h
+++ include/llvm/Support/CommandLine.h
@@ -149,8 +149,8 @@
 public:
   OptionCategory(const char *const Name, const char *const Description = 0)
       : Name(Name), Description(Description) { registerCategory(); }
-  const char *getName() { return Name; }
-  const char *getDescription() { return Description; }
+  const char *getName() const { return Name; }
+  const char *getDescription() const { return Description; }
 };
 
 // The general Option Category (used as default category).
Index: lib/Support/CommandLine.cpp
===================================================================
--- lib/Support/CommandLine.cpp
+++ lib/Support/CommandLine.cpp
@@ -125,8 +125,21 @@
 // Initialise the general option category.
 OptionCategory llvm::cl::GeneralCategory("General options");
 
+struct HasName {
+  HasName(StringRef Name) : Name(Name) {}
+  bool operator()(const OptionCategory *Category) const {
+    return Name == Category->getName();
+  }
+  StringRef Name;
+};
+
 void OptionCategory::registerCategory()
 {
+  assert(std::count_if(RegisteredOptionCategories->begin(),
+                       RegisteredOptionCategories->end(),
+                       HasName(getName())) == 0 &&
+         "Duplicate option categories");
+
   RegisteredOptionCategories->insert(this);
 }
 
@@ -1495,9 +1508,7 @@
   // It shall return true if A's name should be lexographically
   // ordered before B's name. It returns false otherwise.
   static bool OptionCategoryCompare(OptionCategory *A, OptionCategory *B) {
-    int Length = strcmp(A->getName(), B->getName());
-    assert(Length != 0 && "Duplicate option categories");
-    return Length < 0;
+    return strcmp(A->getName(), B->getName());
   }
 
   // Make sure we inherit our base class's operator=()

Index: include/llvm/Support/CommandLine.h
===================================================================
--- include/llvm/Support/CommandLine.h
+++ include/llvm/Support/CommandLine.h
@@ -149,8 +149,8 @@
 public:
   OptionCategory(const char *const Name, const char *const Description = 0)
       : Name(Name), Description(Description) { registerCategory(); }
-  const char *getName() { return Name; }
-  const char *getDescription() { return Description; }
+  const char *getName() const { return Name; }
+  const char *getDescription() const { return Description; }
 };
 
 // The general Option Category (used as default category).
Index: lib/Support/CommandLine.cpp
===================================================================
--- lib/Support/CommandLine.cpp
+++ lib/Support/CommandLine.cpp
@@ -125,8 +125,21 @@
 // Initialise the general option category.
 OptionCategory llvm::cl::GeneralCategory("General options");
 
+struct HasName {
+  HasName(StringRef Name) : Name(Name) {}
+  bool operator()(const OptionCategory *Category) const {
+    return Name == Category->getName();
+  }
+  StringRef Name;
+};
+
 void OptionCategory::registerCategory()
 {
+  assert(std::count_if(RegisteredOptionCategories->begin(),
+                       RegisteredOptionCategories->end(),
+                       HasName(getName())) == 0 &&
+         "Duplicate option categories");
+
   RegisteredOptionCategories->insert(this);
 }
 
@@ -1495,9 +1508,7 @@
   // It shall return true if A's name should be lexographically
   // ordered before B's name. It returns false otherwise.
   static bool OptionCategoryCompare(OptionCategory *A, OptionCategory *B) {
-    int Length = strcmp(A->getName(), B->getName());
-    assert(Length != 0 && "Duplicate option categories");
-    return Length < 0;
+    return strcmp(A->getName(), B->getName());
   }
 
   // Make sure we inherit our base class's operator=()

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits