From a2037284d41905968965638957ea29f9a7e3220e Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Fri, 21 Dec 2018 03:16:30 +0000 Subject: [PATCH] Implement LWG 2936: Path comparison is defined in terms of the generic format This patch implements path::compare according to the current spec. The only observable change is the ordering of "/foo" and "foo", which orders the two paths based on having or not having a root directory (instead of lexically comparing "/" to "foo"). git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@349881 91177308-0d34-0410-b5e6-96231b3b80d8 --- src/filesystem/operations.cpp | 99 ++++++++++++++++--- .../path.member/path.compare.pass.cpp | 55 ++++++++++- www/cxx2a_status.html | 2 +- 3 files changed, 140 insertions(+), 16 deletions(-) diff --git a/src/filesystem/operations.cpp b/src/filesystem/operations.cpp index c9396b59c..0b79ef1cd 100644 --- a/src/filesystem/operations.cpp +++ b/src/filesystem/operations.cpp @@ -206,8 +206,20 @@ public: return *this; } + bool atEnd() const noexcept { + return State == PS_AtEnd; + } + + bool inRootDir() const noexcept { + return State == PS_InRootDir; + } + + bool inRootName() const noexcept { + return State == PS_InRootName; + } + bool inRootPath() const noexcept { - return State == PS_InRootDir || State == PS_InRootName; + return inRootName() || inRootDir(); } private: @@ -1294,7 +1306,19 @@ string_view_t path::__root_path_raw() const { return {}; } +static bool ConsumeRootName(PathParser *PP) { + static_assert(PathParser::PS_BeforeBegin == 1 && + PathParser::PS_InRootName == 2, + "Values for enums are incorrect"); + while (PP->State <= PathParser::PS_InRootName) + ++(*PP); + return PP->State == PathParser::PS_AtEnd; +} + static bool ConsumeRootDir(PathParser* PP) { + static_assert(PathParser::PS_BeforeBegin == 1 && + PathParser::PS_InRootName == 2 && + PathParser::PS_InRootDir == 3, "Values for enums are incorrect"); while (PP->State <= PathParser::PS_InRootDir) ++(*PP); return PP->State == PathParser::PS_AtEnd; @@ -1514,21 +1538,68 @@ path path::lexically_relative(const path& base) const { //////////////////////////////////////////////////////////////////////////// // path.comparisons -int path::__compare(string_view_t __s) const { - auto PP = PathParser::CreateBegin(__pn_); - auto PP2 = PathParser::CreateBegin(__s); - while (PP && PP2) { - int res = (*PP).compare(*PP2); - if (res != 0) - return res; - ++PP; - ++PP2; - } - if (PP.State == PP2.State && !PP) +static int CompareRootName(PathParser *LHS, PathParser *RHS) { + if (!LHS->inRootName() && !RHS->inRootName()) return 0; - if (!PP) + + auto GetRootName = [](PathParser *Parser) -> string_view_t { + return Parser->inRootName() ? **Parser : ""; + }; + int res = GetRootName(LHS).compare(GetRootName(RHS)); + ConsumeRootName(LHS); + ConsumeRootName(RHS); + return res; +} + +static int CompareRootDir(PathParser *LHS, PathParser *RHS) { + if (!LHS->inRootDir() && RHS->inRootDir()) return -1; - return 1; + else if (LHS->inRootDir() && !RHS->inRootDir()) + return 1; + else { + ConsumeRootDir(LHS); + ConsumeRootDir(RHS); + return 0; + } +} + +static int CompareRelative(PathParser *LHSPtr, PathParser *RHSPtr) { + auto &LHS = *LHSPtr; + auto &RHS = *RHSPtr; + + int res; + while (LHS && RHS) { + if ((res = (*LHS).compare(*RHS)) != 0) + return res; + ++LHS; + ++RHS; + } + return 0; +} + +static int CompareEndState(PathParser *LHS, PathParser *RHS) { + if (LHS->atEnd() && !RHS->atEnd()) + return -1; + else if (!LHS->atEnd() && RHS->atEnd()) + return 1; + return 0; +} + +int path::__compare(string_view_t __s) const { + auto LHS = PathParser::CreateBegin(__pn_); + auto RHS = PathParser::CreateBegin(__s); + int res; + + if ((res = CompareRootName(&LHS, &RHS)) != 0) + return res; + + if ((res = CompareRootDir(&LHS, &RHS)) != 0) + return res; + + if ((res = CompareRelative(&LHS, &RHS)) != 0) + return res; + + return CompareEndState(&LHS, &RHS); } //////////////////////////////////////////////////////////////////////////// diff --git a/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp b/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp index 7791097dc..2be6122a0 100644 --- a/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp +++ b/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp @@ -65,6 +65,8 @@ const PathCompareTest CompareTestCases[] = {"//foo//bar///baz////", "//foo/bar/baz/", 0}, // duplicate separators {"///foo/bar", "/foo/bar", 0}, // "///" is not a root directory {"/foo/bar/", "/foo/bar", 1}, // trailing separator + {"foo", "/foo", -1}, // if !this->has_root_directory() and p.has_root_directory(), a value less than 0. + {"/foo", "foo", 1}, // if this->has_root_directory() and !p.has_root_directory(), a value greater than 0. {"//" LONGA "////" LONGB "/" LONGC "///" LONGD, "//" LONGA "/" LONGB "/" LONGC "/" LONGD, 0}, { LONGA "/" LONGB "/" LONGC, LONGA "/" LONGB "/" LONGB, 1} @@ -79,7 +81,7 @@ static inline int normalize_ret(int ret) return ret < 0 ? -1 : (ret > 0 ? 1 : 0); } -int main() +void test_compare_basic() { using namespace fs; for (auto const & TC : CompareTestCases) { @@ -136,3 +138,54 @@ int main() } } } + +int CompareElements(std::vector const& LHS, std::vector const& RHS) { + bool IsLess = std::lexicographical_compare(LHS.begin(), LHS.end(), RHS.begin(), RHS.end()); + if (IsLess) + return -1; + + bool IsGreater = std::lexicographical_compare(RHS.begin(), RHS.end(), LHS.begin(), LHS.end()); + if (IsGreater) + return 1; + + return 0; +} + +void test_compare_elements() { + struct { + std::vector LHSElements; + std::vector RHSElements; + int Expect; + } TestCases[] = { + {{"a"}, {"a"}, 0}, + {{"a"}, {"b"}, -1}, + {{"b"}, {"a"}, 1}, + {{"a", "b", "c"}, {"a", "b", "c"}, 0}, + {{"a", "b", "c"}, {"a", "b", "d"}, -1}, + {{"a", "b", "d"}, {"a", "b", "c"}, 1}, + {{"a", "b"}, {"a", "b", "c"}, -1}, + {{"a", "b", "c"}, {"a", "b"}, 1}, + + }; + + auto BuildPath = [](std::vector const& Elems) { + fs::path p; + for (auto &E : Elems) + p /= E; + return p; + }; + + for (auto &TC : TestCases) { + fs::path LHS = BuildPath(TC.LHSElements); + fs::path RHS = BuildPath(TC.RHSElements); + const int ExpectCmp = CompareElements(TC.LHSElements, TC.RHSElements); + assert(ExpectCmp == TC.Expect); + const int GotCmp = normalize_ret(LHS.compare(RHS)); + assert(GotCmp == TC.Expect); + } +} + +int main() { + test_compare_basic(); + test_compare_elements(); +} diff --git a/www/cxx2a_status.html b/www/cxx2a_status.html index f8a5a831f..25e6cdfe4 100644 --- a/www/cxx2a_status.html +++ b/www/cxx2a_status.html @@ -258,7 +258,7 @@ 2682filesystem::copy() won't create a symlink to a directorySan DiegoNothing to do 2697[concurr.ts] Behavior of future/shared_future unwrapping constructor when given an invalid futureSan Diego 2797Trait precondition violationsSan DiegoResolved by 1285R0 - 2936Path comparison is defined in terms of the generic formatSan Diego + 2936Path comparison is defined in terms of the generic formatSan DiegoComplete 2943Problematic specification of the wide version of basic_filebuf::openSan DiegoNothing to do 2960[fund.ts.v3] nonesuch is insufficiently uselessSan Diego 2995basic_stringbuf default constructor forbids it from using SSO capacitySan Diego