Nice!
On Sun, Jan 15, 2023 at 12:10 PM Benjamin Kramer via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > > 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")); > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
