[clang-tools-extra] b283ae7 - [ADT] Add locale-independent isSpace() to StringExtras. NFC

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Sat May 2 06:20:24 PDT 2020


Author: Sam McCall
Date: 2020-05-02T15:20:05+02:00
New Revision: b283ae7af8261c994a62b17b0eaf90cf649228fe

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

LOG: [ADT] Add locale-independent isSpace() to StringExtras. NFC

Use this in clangd, will follow up with replacements for isspace where
locale-dependent is clearly not intended.

Added: 
    

Modified: 
    clang-tools-extra/clangd/support/Markup.cpp
    llvm/include/llvm/ADT/StringExtras.h
    llvm/unittests/ADT/StringExtrasTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/support/Markup.cpp b/clang-tools-extra/clangd/support/Markup.cpp
index 94081b399a20..9dffdf675d3e 100644
--- a/clang-tools-extra/clangd/support/Markup.cpp
+++ b/clang-tools-extra/clangd/support/Markup.cpp
@@ -26,13 +26,6 @@ namespace clangd {
 namespace markup {
 namespace {
 
-/// Like std::isspace but for "C" locale.
-/// FIXME: move to StringExtras?
-bool isSpace(char C) {
-  return C == ' ' || C == '\f' || C == '\n' || C == '\r' || C == '\t' ||
-         C == '\v';
-}
-
 // Is <contents a plausible start to an HTML tag?
 // Contents may not be the rest of the line, but it's the rest of the plain
 // text, so we expect to see at least the tag name.
@@ -50,11 +43,11 @@ bool looksLikeTag(llvm::StringRef Contents) {
                  .drop_while([](char C) {
                    return llvm::isAlnum(C) || C == '-' || C == '_' || C == ':';
                  })
-                 .drop_while(isSpace);
+                 .drop_while(llvm::isSpace);
   // The rest of the tag consists of attributes, which have restrictive names.
   // If we hit '=', all bets are off (attribute values can contain anything).
   for (; !Contents.empty(); Contents = Contents.drop_front()) {
-    if (llvm::isAlnum(Contents.front()) || isSpace(Contents.front()))
+    if (llvm::isAlnum(Contents.front()) || llvm::isSpace(Contents.front()))
       continue;
     if (Contents.front() == '>' || Contents.startswith("/>"))
       return true; // May close the tag.
@@ -75,7 +68,7 @@ bool looksLikeTag(llvm::StringRef Contents) {
 // a markdown grammar construct.
 bool needsLeadingEscape(char C, llvm::StringRef Before, llvm::StringRef After,
                         bool StartsLine) {
-  assert(Before.take_while(isSpace).empty());
+  assert(Before.take_while(llvm::isSpace).empty());
   auto RulerLength = [&]() -> /*Length*/ unsigned {
     if (!StartsLine || !Before.empty())
       return false;
@@ -87,8 +80,8 @@ bool needsLeadingEscape(char C, llvm::StringRef Before, llvm::StringRef After,
            (After.empty() || After.startswith(" "));
   };
   auto SpaceSurrounds = [&]() {
-    return (After.empty() || isSpace(After.front())) &&
-           (Before.empty() || isSpace(Before.back()));
+    return (After.empty() || llvm::isSpace(After.front())) &&
+           (Before.empty() || llvm::isSpace(Before.back()));
   };
   auto WordSurrounds = [&]() {
     return (!After.empty() && llvm::isAlnum(After.front())) &&
@@ -434,8 +427,8 @@ Paragraph &Paragraph::appendText(llvm::StringRef Text) {
   Chunk &C = Chunks.back();
   C.Contents = std::move(Norm);
   C.Kind = Chunk::PlainText;
-  C.SpaceBefore = isSpace(Text.front());
-  C.SpaceAfter = isSpace(Text.back());
+  C.SpaceBefore = llvm::isSpace(Text.front());
+  C.SpaceAfter = llvm::isSpace(Text.back());
   return *this;
 }
 

diff  --git a/llvm/include/llvm/ADT/StringExtras.h b/llvm/include/llvm/ADT/StringExtras.h
index 3f73c0f3d456..56d5f3d05857 100644
--- a/llvm/include/llvm/ADT/StringExtras.h
+++ b/llvm/include/llvm/ADT/StringExtras.h
@@ -107,6 +107,14 @@ inline bool isPrint(char C) {
   return (0x20 <= UC) && (UC <= 0x7E);
 }
 
+/// Checks whether character \p C is whitespace in the "C" locale.
+///
+/// Locale-independent version of the C standard library isspace.
+inline bool isSpace(char C) {
+  return C == ' ' || C == '\f' || C == '\n' || C == '\r' || C == '\t' ||
+         C == '\v';
+}
+
 /// Returns the corresponding lowercase character if \p x is uppercase.
 inline char toLower(char x) {
   if (x >= 'A' && x <= 'Z')

diff  --git a/llvm/unittests/ADT/StringExtrasTest.cpp b/llvm/unittests/ADT/StringExtrasTest.cpp
index 681464e7e7c3..67d573d64975 100644
--- a/llvm/unittests/ADT/StringExtrasTest.cpp
+++ b/llvm/unittests/ADT/StringExtrasTest.cpp
@@ -23,6 +23,17 @@ TEST(StringExtrasTest, isPrint) {
   EXPECT_TRUE(isPrint('?'));
 }
 
+TEST(StringExtrasTest, isSpace) {
+  EXPECT_TRUE(isSpace(' '));
+  EXPECT_TRUE(isSpace('\t'));
+  EXPECT_TRUE(isSpace('\n'));
+  EXPECT_TRUE(isSpace('\v'));
+  EXPECT_TRUE(isSpace('\f'));
+  EXPECT_TRUE(isSpace('\v'));
+  EXPECT_FALSE(isSpace('\0'));
+  EXPECT_FALSE(isSpace('_'));
+}
+
 TEST(StringExtrasTest, Join) {
   std::vector<std::string> Items;
   EXPECT_EQ("", join(Items.begin(), Items.end(), " <sep> "));


        


More information about the cfe-commits mailing list