facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2025-01-01 09:37:56 +03:00
Code Issues Projects Releases Wiki Activity

eden: add CaseSensitive template param to PathMap

Browse Source 
Summary:
This commit adds a compile time option to select
between case-sensitive and case-insensitive-but-case-preserving
mode for `PathMap`.

This replaces the `ifdef _WIN32` preprocessor conditional
that was inline in a couple of the methods and allows
explicitly testing the behavior in both modes of operation.

The unit tests have been expanded and rounded out to catch
some inconsistent behavior; insertion wasn't respecting
case insensitivity in all ... cases.

Hopefully we not relying on that behavior in the windows
flavor of the build; let's see what our CI says.

Reviewed By: genevievehelsel

Differential Revision: D23232629

fbshipit-source-id: 96e752e501d0398ec2bed5879f7c11c7ab6e1d70
This commit is contained in:
Wez Furlong 2020-09-02 10:15:38 -07:00 committed by Facebook GitHub Bot
parent 533af0f60b
commit 7efa1b5745
2 changed files with 124 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

108
eden/fs/utils/PathMap.h
View File

@ -7,6 +7,7 @@
#pragma once
#include <folly/FBVector.h>
#include <folly/Portability.h>
#include <algorithm>
#include <functional>
#include <iterator>
@ -16,6 +17,8 @@
namespace facebook {
namespace eden {
constexpr bool kPathMapDefaultCaseSensitive = !folly::kIsWindows;
/** An associative container that maps from one of our path types to an
* arbitrary value type.
*
@ -30,7 +33,10 @@ namespace eden {
* - Since insert and erase operations move the vector contents around,
* those operations invalidate iterators.
*/
template <typename Value, typename Key = PathComponent>
template <
typename Value,
typename Key = PathComponent,
bool CaseSensitive = kPathMapDefaultCaseSensitive>
class PathMap : private folly::fbvector<std::pair<Key, Value>> {
using Pair = std::pair<Key, Value>;
using Vector = folly::fbvector<Pair>;
@ -164,18 +170,17 @@ class PathMap : private folly::fbvector<std::pair<Key, Value>> {
// Found it
return iter;
}
#ifdef _WIN32
// On Windows we need to do a case insensitive lookup for the file and
// directory names. For performance, we will do a case sensitive search
// first which should cover most of the cases and if not found then do a
// case sensitive search.
for (iter = begin(); iter != end(); ++iter) {
if (key.stringPiece().equals(
iter->first.stringPiece(), folly::AsciiCaseInsensitive())) {
return iter;
if (!CaseSensitive) {
// When !CaseSensitive, for performance, we will do a case sensitive
// search first which should cover most of the cases and if not found then
// do a case insensitive search.
for (iter = begin(); iter != end(); ++iter) {
if (key.stringPiece().equals(
iter->first.stringPiece(), folly::AsciiCaseInsensitive())) {
return iter;
}
}
}
#endif
return end();
}
@ -188,18 +193,14 @@ class PathMap : private folly::fbvector<std::pair<Key, Value>> {
// Found it
return iter;
}
#ifdef _WIN32
// On Windows we need to do a case insensitive lookup for the file and
// directory names. For performance, we will do a case sensitive search
// first which should cover most of the cases and if not found then do a
// case sensitive search.
for (iter = begin(); iter != end(); ++iter) {
if (key.stringPiece().equals(
iter->first.stringPiece(), folly::AsciiCaseInsensitive())) {
return iter;
if (!CaseSensitive) {
for (iter = begin(); iter != end(); ++iter) {
if (key.stringPiece().equals(
iter->first.stringPiece(), folly::AsciiCaseInsensitive())) {
return iter;
}
}
}
#endif
return end();
}
@ -209,10 +210,24 @@ class PathMap : private folly::fbvector<std::pair<Key, Value>> {
* a boolean that is true if an insert took place. */
std::pair<iterator, bool> insert(const value_type& val) {
auto iter = lower_bound(val.first);
if (iter == end() || compare_(val.first, iter->first)) {
return std::make_pair(Vector::insert(iter, val), true);
if (iter != end() && !compare_(val.first, iter->first)) {
// Found it; leave it alone
return std::make_pair(iter, false);
}
return std::make_pair(iter, false);
if (!CaseSensitive) {
for (auto insens = begin(); insens != end(); ++insens) {
if (val.first.stringPiece().equals(
insens->first.stringPiece(), folly::AsciiCaseInsensitive())) {
// Found it; leave it alone
return std::make_pair(insens, false);
}
}
}
// Otherwise, iter is the insertion point
return std::make_pair(Vector::insert(iter, val), true);
}
/** Emplace a new key-value pair by constructing it in-place.
@ -224,22 +239,51 @@ class PathMap : private folly::fbvector<std::pair<Key, Value>> {
template <typename... Args>
std::pair<iterator, bool> emplace(Piece key, Args&&... args) {
auto iter = lower_bound(key);
if (iter == end() || compare_(key, iter->first)) {
iter = Vector::emplace(
iter, std::make_pair(Key(key), Value(std::forward<Args>(args)...)));
return std::make_pair(iter, true);
if (iter != end() && !compare_(key, iter->first)) {
// Found it; leave it alone
return std::make_pair(iter, false);
}
return std::make_pair(iter, false);
if (!CaseSensitive) {
for (auto insens = begin(); insens != end(); ++insens) {
if (key.stringPiece().equals(
insens->first.stringPiece(), folly::AsciiCaseInsensitive())) {
// Found it; leave it alone
return std::make_pair(insens, false);
}
}
}
// Otherwise, iter is the insertion point
iter = Vector::emplace(
iter, std::make_pair(Key(key), Value(std::forward<Args>(args)...)));
return std::make_pair(iter, true);
}
/** Returns a reference to the map position for key, creating it needed.
* If the key is already present, no additional allocations are performed. */
mapped_type& operator[](Piece key) {
auto iter = lower_bound(key);
if (iter == end() || compare_(key, iter->first)) {
// Not yet present, make a new one
iter = Vector::insert(iter, std::make_pair(Key(key), mapped_type()));
if (iter != end() && !compare_(key, iter->first)) {
// Found it
return iter->second;
}
if (!CaseSensitive) {
// Case insensitive lookup
for (auto insens = begin(); insens != end(); ++insens) {
if (key.stringPiece().equals(
insens->first.stringPiece(), folly::AsciiCaseInsensitive())) {
// Found it
return insens->second;
}
}
}
// Not yet present, make a new one at the insertion point
iter = Vector::insert(iter, std::make_pair(Key(key), mapped_type()));
return iter->second;
}

48
eden/fs/utils/test/PathMapTest.cpp
View File

@ -14,6 +14,54 @@ using facebook::eden::PathComponentPiece;
using facebook::eden::PathMap;
using namespace facebook::eden::path_literals;
TEST(PathMap, caseSensitive) {
// Explicitly a case sensitive map, regardless of the host OS
PathMap<bool, PathComponent, true> map;
map.insert(std::make_pair(PathComponent("foo"), true));
EXPECT_TRUE(map.at("foo"_pc));
EXPECT_EQ(map.find("Foo"_pc), map.end());
EXPECT_TRUE(map.insert(std::make_pair(PathComponent("FOO"), false)).second);
EXPECT_EQ(map.size(), 2);
EXPECT_TRUE(map.at("foo"_pc));
EXPECT_FALSE(map.at("FOO"_pc));
EXPECT_EQ(map.erase("FOO"_pc), 1);
EXPECT_EQ(map.size(), 1);
map["FOO"_pc] = true;
map["Foo"_pc] = false;
EXPECT_EQ(map.size(), 3);
}
TEST(PathMap, caseInSensitive) {
// Explicitly a case IN-sensitive map, regardless of the host OS
PathMap<bool, PathComponent, false> map;
map.insert(std::make_pair(PathComponent("foo"), true));
EXPECT_TRUE(map.at("foo"_pc));
EXPECT_TRUE(map.at("Foo"_pc));
EXPECT_FALSE(map.insert(std::make_pair(PathComponent("FOO"), false)).second);
EXPECT_FALSE(map.emplace(PathComponent("FOO"), false).second);
EXPECT_EQ(map.size(), 1);
EXPECT_TRUE(map.at("foo"_pc));
EXPECT_TRUE(map.at("FOO"_pc));
EXPECT_EQ(map.erase("FOO"_pc), 1);
EXPECT_EQ(map.size(), 0);
// Case insensitive referencing
map["FOO"_pc] = true;
map["Foo"_pc] = false;
// Only one FOO entry
EXPECT_EQ(map.size(), 1);
// It shows as false
EXPECT_EQ(map["FOO"_pc], false);
// The assignment above didn't change the case of the key!
EXPECT_EQ(map.begin()->first, "FOO"_pc);
}
TEST(PathMap, insert) {
PathMap<bool> map;