[PATCH] isCharInSet refactoring.

Alexander Kornienko alexfh at google.com
Wed Aug 28 04:41:03 PDT 2013


  Updated comments.

Hi jordan_rose, klimek,

http://llvm-reviews.chandlerc.com/D1534

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1534?vs=3821&id=3864#toc

Files:
  include/llvm/Support/UnicodeCharRanges.h
  lib/Support/LocaleGeneric.inc

Index: include/llvm/Support/UnicodeCharRanges.h
===================================================================
--- include/llvm/Support/UnicodeCharRanges.h
+++ include/llvm/Support/UnicodeCharRanges.h
@@ -17,82 +17,80 @@
 #include "llvm/Support/MutexGuard.h"
 #include "llvm/Support/raw_ostream.h"
 
-namespace {
+#include <algorithm>
 
+namespace llvm {
+namespace sys {
+
+/// \brief Represents a closed range of Unicode code points [Lower, Upper].
 struct UnicodeCharRange {
   uint32_t Lower;
   uint32_t Upper;
 };
-typedef llvm::ArrayRef<UnicodeCharRange> UnicodeCharSet;
 
-/// Returns true if each of the ranges in \p CharSet is a proper closed range
-/// [min, max], and if the ranges themselves are ordered and non-overlapping.
-static inline bool isValidCharSet(UnicodeCharSet CharSet) {
-#ifndef NDEBUG
-  static llvm::SmallPtrSet<const UnicodeCharRange *, 16> Validated;
-  static llvm::sys::Mutex ValidationMutex;
-
-  // Check the validation cache.
-  {
-    llvm::MutexGuard Guard(ValidationMutex);
-    if (Validated.count(CharSet.data()))
-      return true;
-  }
+inline bool operator<(uint32_t Value, UnicodeCharRange Range) {
+  return Value < Range.Lower;
+}
+inline bool operator<(UnicodeCharRange Range, uint32_t Value) {
+  return Range.Upper < Value;
+}
 
-  // Walk through the ranges.
-  uint32_t Prev = 0;
-  for (UnicodeCharSet::iterator I = CharSet.begin(), E = CharSet.end();
-       I != E; ++I) {
-    if (I != CharSet.begin() && Prev >= I->Lower) {
-      DEBUG(llvm::dbgs() << "Upper bound 0x");
-      DEBUG(llvm::dbgs().write_hex(Prev));
-      DEBUG(llvm::dbgs() << " should be less than succeeding lower bound 0x");
-      DEBUG(llvm::dbgs().write_hex(I->Lower) << "\n");
-      return false;
-    }
-    if (I->Upper < I->Lower) {
-      DEBUG(llvm::dbgs() << "Upper bound 0x");
-      DEBUG(llvm::dbgs().write_hex(I->Lower));
-      DEBUG(llvm::dbgs() << " should not be less than lower bound 0x");
-      DEBUG(llvm::dbgs().write_hex(I->Upper) << "\n");
-      return false;
-    }
-    Prev = I->Upper;
+/// \brief Holds a reference to an ordered array of UnicodeCharRange and allows
+/// to quickly check if a code point is contained in the set represented by this
+/// array.
+class UnicodeCharSet {
+public:
+  typedef llvm::ArrayRef<UnicodeCharRange> CharRanges;
+
+  /// \brief Constructs a UnicodeCharSet instance from an array of
+  /// UnicodeCharRanges.
+  ///
+  /// Array pointed by \p Ranges should have the lifetime at least as long as
+  /// the UnicodeCharSet instance, and should not change. Array is validated by
+  /// the constructor, so it makes sense to create as few UnicodeCharSet
+  /// instances per each array of ranges, as possible.
+  UnicodeCharSet(CharRanges Ranges) : Ranges(Ranges) {
+    assert(rangesAreValid());
   }
 
-  // Update the validation cache.
-  {
-    llvm::MutexGuard Guard(ValidationMutex);
-    Validated.insert(CharSet.data());
+  /// \brief Returns true if the character set contains the Unicode code point
+  /// \p C.
+  bool contains(uint32_t C) const {
+    return std::binary_search(Ranges.begin(), Ranges.end(), C);
   }
-#endif
-  return true;
-}
-
-} // namespace
 
+private:
+  /// \brief Returns true if each of the ranges is a proper closed range
+  /// [min, max], and if the ranges themselves are ordered and non-overlapping.
+  bool rangesAreValid() const {
+    uint32_t Prev = 0;
+    for (CharRanges::const_iterator I = Ranges.begin(), E = Ranges.end();
+         I != E; ++I) {
+      if (I != Ranges.begin() && Prev >= I->Lower) {
+        DEBUG(llvm::dbgs() << "Upper bound 0x");
+        DEBUG(llvm::dbgs().write_hex(Prev));
+        DEBUG(llvm::dbgs() << " should be less than succeeding lower bound 0x");
+        DEBUG(llvm::dbgs().write_hex(I->Lower) << "\n");
+        return false;
+      }
+      if (I->Upper < I->Lower) {
+        DEBUG(llvm::dbgs() << "Upper bound 0x");
+        DEBUG(llvm::dbgs().write_hex(I->Lower));
+        DEBUG(llvm::dbgs() << " should not be less than lower bound 0x");
+        DEBUG(llvm::dbgs().write_hex(I->Upper) << "\n");
+        return false;
+      }
+      Prev = I->Upper;
+    }
 
-/// Returns true if the Unicode code point \p C is within the set of
-/// characters specified by \p CharSet.
-LLVM_READONLY static inline bool isCharInSet(uint32_t C,
-                                             UnicodeCharSet CharSet) {
-  assert(isValidCharSet(CharSet));
-
-  size_t LowPoint = 0;
-  size_t HighPoint = CharSet.size();
-
-  // Binary search the set of char ranges.
-  while (HighPoint != LowPoint) {
-    size_t MidPoint = (HighPoint + LowPoint) / 2;
-    if (C < CharSet[MidPoint].Lower)
-      HighPoint = MidPoint;
-    else if (C > CharSet[MidPoint].Upper)
-      LowPoint = MidPoint + 1;
-    else
-      return true;
+    return true;
   }
 
-  return false;
-}
+  const CharRanges Ranges;
+};
+
+} // namespace sys
+} // namespace llvm
+
 
 #endif // LLVM_SUPPORT_UNICODECHARRANGES_H
Index: lib/Support/LocaleGeneric.inc
===================================================================
--- lib/Support/LocaleGeneric.inc
+++ lib/Support/LocaleGeneric.inc
@@ -229,8 +229,9 @@
     { 0x2B735, 0x2B73F }, { 0x2B81E, 0x2F7FF }, { 0x2FA1E, 0xF0000 },
     { 0xFFFFE, 0xFFFFF }, { 0x10FFFE, 0x10FFFF }
   };
+  static const UnicodeCharSet NonPrintables(NonPrintableRanges);
 
-  return UCS >= 0 && UCS <= 0x10FFFF && !isCharInSet(UCS, NonPrintableRanges);
+  return UCS >= 0 && UCS <= 0x10FFFF && !NonPrintables.contains(UCS);
 }
 
 /// Gets the number of positions a character is likely to occupy when output
@@ -252,7 +253,7 @@
   // Sorted list of non-spacing and enclosing combining mark intervals as
   // defined in "3.6 Combination" of
   // http://www.unicode.org/versions/Unicode6.2.0/UnicodeStandard-6.2.pdf
-  static const UnicodeCharRange CombiningCharacters[] = {
+  static const UnicodeCharRange CombiningCharacterRanges[] = {
     { 0x0300, 0x036F }, { 0x0483, 0x0489 }, { 0x0591, 0x05BD },
     { 0x05BF, 0x05BF }, { 0x05C1, 0x05C2 }, { 0x05C4, 0x05C5 },
     { 0x05C7, 0x05C7 }, { 0x0610, 0x061A }, { 0x064B, 0x065F },
@@ -327,11 +328,12 @@
     { 0x1D17B, 0x1D182 }, { 0x1D185, 0x1D18B }, { 0x1D1AA, 0x1D1AD },
     { 0x1D242, 0x1D244 }, { 0xE0100, 0xE01EF },
   };
+  static const UnicodeCharSet CombiningCharacters(CombiningCharacterRanges);
 
-  if (isCharInSet(UCS, CombiningCharacters))
+  if (CombiningCharacters.contains(UCS))
     return 0;
 
-  static const UnicodeCharRange DoubleWidthCharacters[] = {
+  static const UnicodeCharRange DoubleWidthCharacterRanges[] = {
     // Hangul Jamo
     { 0x1100, 0x11FF },
     // Deprecated fullwidth angle brackets
@@ -352,8 +354,9 @@
     // CJK Unified Ideographs
     { 0x20000, 0x2A6DF }, { 0x2A700, 0x2B81F }, { 0x2F800, 0x2FA1F }
   };
+  static const UnicodeCharSet DoubleWidthCharacters(DoubleWidthCharacterRanges);
 
-  if (isCharInSet(UCS, DoubleWidthCharacters))
+  if (DoubleWidthCharacters.contains(UCS))
     return 2;
   return 1;
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1534.2.patch
Type: text/x-patch
Size: 6995 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130828/3bebdd15/attachment.bin>


More information about the cfe-commits mailing list