[clang] 931d04b - [ADT] Make StringRef::compare like std::string_view::compare
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 16 22:05:47 PST 2023
Nice!
On Sun, Jan 15, 2023 at 12:10 PM Benjamin Kramer via cfe-commits
<cfe-commits at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list