[lld] 931d04b - [ADT] Make StringRef::compare like std::string_view::compare

Benjamin Kramer via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 15 12:10:35 PST 2023


Author: Benjamin Kramer
Date: 2023-01-15T20:59:21+01:00
New Revision: 931d04be2fc8f3f0505b43e64297f75d526cb42a

URL: https://github.com/llvm/llvm-project/commit/931d04be2fc8f3f0505b43e64297f75d526cb42a
DIFF: https://github.com/llvm/llvm-project/commit/931d04be2fc8f3f0505b43e64297f75d526cb42a.diff

LOG: [ADT] Make StringRef::compare like std::string_view::compare

string_view has a slightly weaker contract, which only specifies whether
the value is bigger or smaller than 0. Adapt users accordingly and just
forward to the standard function (that also compiles down to memcmp)

Added: 
    

Modified: 
    clang/lib/AST/DeclarationName.cpp
    clang/lib/Analysis/PathDiagnostic.cpp
    clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
    clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
    lld/COFF/Writer.cpp
    llvm/include/llvm/ADT/SmallString.h
    llvm/include/llvm/ADT/StringRef.h
    llvm/include/llvm/ProfileData/SampleProf.h
    llvm/lib/Transforms/IPO/MergeFunctions.cpp
    llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
    llvm/unittests/ADT/SmallStringTest.cpp
    llvm/unittests/ADT/StringRefTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/DeclarationName.cpp b/clang/lib/AST/DeclarationName.cpp
index b2232ddfced32..c1219041a466b 100644
--- a/clang/lib/AST/DeclarationName.cpp
+++ b/clang/lib/AST/DeclarationName.cpp
@@ -72,15 +72,9 @@ int DeclarationName::compare(DeclarationName LHS, DeclarationName RHS) {
     }
     unsigned LN = LHSSelector.getNumArgs(), RN = RHSSelector.getNumArgs();
     for (unsigned I = 0, N = std::min(LN, RN); I != N; ++I) {
-      switch (LHSSelector.getNameForSlot(I).compare(
-          RHSSelector.getNameForSlot(I))) {
-      case -1:
-        return -1;
-      case 1:
-        return 1;
-      default:
-        break;
-      }
+      if (int Compare = LHSSelector.getNameForSlot(I).compare(
+              RHSSelector.getNameForSlot(I)))
+        return Compare;
     }
 
     return compareInt(LN, RN);

diff  --git a/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp
index 9e1215fe3d01d..ac1306fd80711 100644
--- a/clang/lib/Analysis/PathDiagnostic.cpp
+++ b/clang/lib/Analysis/PathDiagnostic.cpp
@@ -342,7 +342,7 @@ static bool compareCrossTUSourceLocs(FullSourceLoc XL, FullSourceLoc YL) {
     return XFE && !YFE;
   int NameCmp = XFE->getName().compare(YFE->getName());
   if (NameCmp != 0)
-    return NameCmp == -1;
+    return NameCmp < 0;
   // Last resort: Compare raw file IDs that are possibly expansions.
   return XL.getFileID() < YL.getFileID();
 }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index b38d18d3691d9..12b948a65261f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2146,7 +2146,7 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
         DefinedSVal zeroVal = svalBuilder.makeIntVal(0, CE->getType());
         // Constrain strcmp's result range based on the result of StringRef's
         // comparison methods.
-        BinaryOperatorKind op = (compareRes == 1) ? BO_GT : BO_LT;
+        BinaryOperatorKind op = (compareRes > 0) ? BO_GT : BO_LT;
         SVal compareWithZero =
           svalBuilder.evalBinOp(state, op, resultVal, zeroVal,
               svalBuilder.getConditionType());

diff  --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 1fb4c83052c4a..1daa58f20fd5b 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -1272,8 +1272,8 @@ linkAndWrapDeviceFiles(SmallVectorImpl<OffloadFile> &LinkerInputFiles,
     // We sort the entries before bundling so they appear in a deterministic
     // order in the final binary.
     llvm::sort(Input, [](OffloadingImage &A, OffloadingImage &B) {
-      return A.StringData["triple"].compare(B.StringData["triple"]) == 1 ||
-             A.StringData["arch"].compare(B.StringData["arch"]) == 1 ||
+      return A.StringData["triple"] > B.StringData["triple"] ||
+             A.StringData["arch"] > B.StringData["arch"] ||
              A.TheOffloadKind < B.TheOffloadKind;
     });
     auto BundledImagesOrErr = bundleLinkedOutput(Input, Args, Kind);

diff  --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 09cca5667a470..b02ad01bdef74 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -188,7 +188,7 @@ class PartialSectionKey {
 
   bool operator<(const PartialSectionKey &other) const {
     int c = name.compare(other.name);
-    if (c == 1)
+    if (c > 0)
       return false;
     if (c == 0)
       return characteristics < other.characteristics;

diff  --git a/llvm/include/llvm/ADT/SmallString.h b/llvm/include/llvm/ADT/SmallString.h
index 874968f0a13f3..0052c86fb37b8 100644
--- a/llvm/include/llvm/ADT/SmallString.h
+++ b/llvm/include/llvm/ADT/SmallString.h
@@ -98,8 +98,9 @@ class SmallString : public SmallVector<char, InternalLen> {
     return str().equals_insensitive(RHS);
   }
 
-  /// Compare two strings; the result is -1, 0, or 1 if this string is
-  /// lexicographically less than, equal to, or greater than the \p RHS.
+  /// compare - Compare two strings; the result is negative, zero, or positive
+  /// if this string is lexicographically less than, equal to, or greater than
+  /// the \p RHS.
   int compare(StringRef RHS) const {
     return str().compare(RHS);
   }

diff  --git a/llvm/include/llvm/ADT/StringRef.h b/llvm/include/llvm/ADT/StringRef.h
index 9fea390c2ed32..f156f744fda11 100644
--- a/llvm/include/llvm/ADT/StringRef.h
+++ b/llvm/include/llvm/ADT/StringRef.h
@@ -171,17 +171,11 @@ namespace llvm {
       return Length == RHS.Length && compare_insensitive(RHS) == 0;
     }
 
-    /// compare - Compare two strings; the result is -1, 0, or 1 if this string
-    /// is lexicographically less than, equal to, or greater than the \p RHS.
+    /// compare - Compare two strings; the result is negative, zero, or positive
+    /// if this string is lexicographically less than, equal to, or greater than
+    /// the \p RHS.
     [[nodiscard]] int compare(StringRef RHS) const {
-      // Check the prefix for a mismatch.
-      if (int Res = compareMemory(Data, RHS.Data, std::min(Length, RHS.Length)))
-        return Res < 0 ? -1 : 1;
-
-      // Otherwise the prefixes match, so we only need to check the lengths.
-      if (Length == RHS.Length)
-        return 0;
-      return Length < RHS.Length ? -1 : 1;
+      return std::string_view(*this).compare(RHS);
     }
 
     /// Compare two strings, ignoring case.
@@ -878,19 +872,19 @@ namespace llvm {
   inline bool operator!=(StringRef LHS, StringRef RHS) { return !(LHS == RHS); }
 
   inline bool operator<(StringRef LHS, StringRef RHS) {
-    return LHS.compare(RHS) == -1;
+    return LHS.compare(RHS) < 0;
   }
 
   inline bool operator<=(StringRef LHS, StringRef RHS) {
-    return LHS.compare(RHS) != 1;
+    return LHS.compare(RHS) <= 0;
   }
 
   inline bool operator>(StringRef LHS, StringRef RHS) {
-    return LHS.compare(RHS) == 1;
+    return LHS.compare(RHS) > 0;
   }
 
   inline bool operator>=(StringRef LHS, StringRef RHS) {
-    return LHS.compare(RHS) != -1;
+    return LHS.compare(RHS) >= 0;
   }
 
   inline std::string &operator+=(std::string &buffer, StringRef string) {

diff  --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index db101b7bc7a87..13f0157222eca 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -648,7 +648,7 @@ class SampleContext {
       return State < That.State;
 
     if (!hasContext()) {
-      return (Name.compare(That.Name)) == -1;
+      return Name < That.Name;
     }
 
     uint64_t I = 0;
@@ -657,7 +657,7 @@ class SampleContext {
       auto &Context2 = That.FullContext[I];
       auto V = Context1.FuncName.compare(Context2.FuncName);
       if (V)
-        return V == -1;
+        return V < 0;
       if (Context1.Location != Context2.Location)
         return Context1.Location < Context2.Location;
       I++;

diff  --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index f4ac497fece67..590f62ca58dd9 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -215,7 +215,7 @@ class MergeFunctions {
       if (LHS.getHash() != RHS.getHash())
         return LHS.getHash() < RHS.getHash();
       FunctionComparator FCmp(LHS.getFunc(), RHS.getFunc(), GlobalNumbers);
-      return FCmp.compare() == -1;
+      return FCmp.compare() < 0;
     }
   };
   using FnTreeType = std::set<FunctionNode, FunctionNodeCmp>;

diff  --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
index 397588278c01c..20f18322d43cc 100644
--- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -511,7 +511,8 @@ Value *LibCallSimplifier::optimizeStrCmp(CallInst *CI, IRBuilderBase &B) {
 
   // strcmp(x, y)  -> cnst  (if both x and y are constant strings)
   if (HasStr1 && HasStr2)
-    return ConstantInt::get(CI->getType(), Str1.compare(Str2));
+    return ConstantInt::get(CI->getType(),
+                            std::clamp(Str1.compare(Str2), -1, 1));
 
   if (HasStr1 && Str1.empty()) // strcmp("", x) -> -*x
     return B.CreateNeg(B.CreateZExt(
@@ -595,7 +596,8 @@ Value *LibCallSimplifier::optimizeStrNCmp(CallInst *CI, IRBuilderBase &B) {
     // Avoid truncating the 64-bit Length to 32 bits in ILP32.
     StringRef SubStr1 = substr(Str1, Length);
     StringRef SubStr2 = substr(Str2, Length);
-    return ConstantInt::get(CI->getType(), SubStr1.compare(SubStr2));
+    return ConstantInt::get(CI->getType(),
+                            std::clamp(SubStr1.compare(SubStr2), -1, 1));
   }
 
   if (HasStr1 && Str1.empty()) // strncmp("", x, n) -> -*x

diff  --git a/llvm/unittests/ADT/SmallStringTest.cpp b/llvm/unittests/ADT/SmallStringTest.cpp
index b207f582e9197..2f4df8afeafa5 100644
--- a/llvm/unittests/ADT/SmallStringTest.cpp
+++ b/llvm/unittests/ADT/SmallStringTest.cpp
@@ -203,12 +203,12 @@ TEST_F(SmallStringTest, Realloc) {
 }
 
 TEST_F(SmallStringTest, Comparisons) {
-  EXPECT_EQ(-1, SmallString<10>("aab").compare("aad"));
+  EXPECT_GT( 0, SmallString<10>("aab").compare("aad"));
   EXPECT_EQ( 0, SmallString<10>("aab").compare("aab"));
-  EXPECT_EQ( 1, SmallString<10>("aab").compare("aaa"));
-  EXPECT_EQ(-1, SmallString<10>("aab").compare("aabb"));
-  EXPECT_EQ( 1, SmallString<10>("aab").compare("aa"));
-  EXPECT_EQ( 1, SmallString<10>("\xFF").compare("\1"));
+  EXPECT_LT( 0, SmallString<10>("aab").compare("aaa"));
+  EXPECT_GT( 0, SmallString<10>("aab").compare("aabb"));
+  EXPECT_LT( 0, SmallString<10>("aab").compare("aa"));
+  EXPECT_LT( 0, SmallString<10>("\xFF").compare("\1"));
 
   EXPECT_EQ(-1, SmallString<10>("AaB").compare_insensitive("aAd"));
   EXPECT_EQ( 0, SmallString<10>("AaB").compare_insensitive("aab"));

diff  --git a/llvm/unittests/ADT/StringRefTest.cpp b/llvm/unittests/ADT/StringRefTest.cpp
index 4ea1ea5455185..cad1974dd1263 100644
--- a/llvm/unittests/ADT/StringRefTest.cpp
+++ b/llvm/unittests/ADT/StringRefTest.cpp
@@ -81,12 +81,12 @@ TEST(StringRefTest, StringOps) {
   EXPECT_EQ(p, StringRef(p, 0).data());
   EXPECT_TRUE(StringRef().empty());
   EXPECT_EQ((size_t) 5, StringRef("hello").size());
-  EXPECT_EQ(-1, StringRef("aab").compare("aad"));
+  EXPECT_GT( 0, StringRef("aab").compare("aad"));
   EXPECT_EQ( 0, StringRef("aab").compare("aab"));
-  EXPECT_EQ( 1, StringRef("aab").compare("aaa"));
-  EXPECT_EQ(-1, StringRef("aab").compare("aabb"));
-  EXPECT_EQ( 1, StringRef("aab").compare("aa"));
-  EXPECT_EQ( 1, StringRef("\xFF").compare("\1"));
+  EXPECT_LT( 0, StringRef("aab").compare("aaa"));
+  EXPECT_GT( 0, StringRef("aab").compare("aabb"));
+  EXPECT_LT( 0, StringRef("aab").compare("aa"));
+  EXPECT_LT( 0, StringRef("\xFF").compare("\1"));
 
   EXPECT_EQ(-1, StringRef("AaB").compare_insensitive("aAd"));
   EXPECT_EQ( 0, StringRef("AaB").compare_insensitive("aab"));


        


More information about the llvm-commits mailing list