Fixed testing Symbol is a valid constant.

rubinius · Apr 30, 2015 · 91cf758 · 91cf758
1 parent b9bd788
commit 91cf758
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 35 deletions.
diff --git a/spec/ruby/core/module/const_defined_spec.rb b/spec/ruby/core/module/const_defined_spec.rb
@@ -81,6 +81,12 @@
     ConstantSpecs.const_defined?("CS_CONSTλ").should be_true
   end
 
+  it "returns true when passed a scoped constant name with EUC-JP characters" do
+    str = "CS_CONSTλ".encode("euc-jp")
+    ConstantSpecs.const_set str, 1
+    ConstantSpecs.const_defined?(str).should be_true
+  end
+
   it "returns true when passed a scoped constant name for a constant in the inheritance hierarchy and the inherited flag is default" do
     ConstantSpecs::ClassD.const_defined?("ClassE::CS_CONST2").should be_true
   end

diff --git a/vm/builtin/symbol.cpp b/vm/builtin/symbol.cpp
@@ -60,15 +60,15 @@ namespace rubinius {
   }
 
   Object* Symbol::is_ivar_p(STATE) {
-    return RBOOL(state->shared().symbols.kind(state, this) == SymbolTable::IVar);
+    return RBOOL(state->shared().symbols.kind(state, this) == SymbolTable::eIVar);
   }
 
   Object* Symbol::is_cvar_p(STATE) {
-    return RBOOL(state->shared().symbols.kind(state, this) == SymbolTable::CVar);
+    return RBOOL(state->shared().symbols.kind(state, this) == SymbolTable::eCVar);
   }
 
   Object* Symbol::is_constant_p(STATE) {
-    return RBOOL(state->shared().symbols.kind(state, this) == SymbolTable::Constant);
+    return RBOOL(state->shared().symbols.kind(state, this) == SymbolTable::eConstant);
   }
 
   Encoding* Symbol::encoding(STATE) {

diff --git a/vm/symbol_table.cpp b/vm/symbol_table.cpp
@@ -1,3 +1,7 @@
+#include "oniguruma.h" // Must be first.
+#include "regenc.h"
+#include "transcoder.h"
+
 #include "symbol_table.hpp"
 #include "exception.hpp"
 #include "configuration.hpp"
@@ -13,49 +17,78 @@
 
 namespace rubinius {
 
-  SymbolTable::Kind SymbolTable::detect_kind(const char* str, size_t size) {
-    const char one = str[0];
+  SymbolTable::Kind SymbolTable::detect_kind(STATE, const Symbol* sym) {
+    std::string str = strings[sym->index()];
+    size_t size = str.size();
+    uint8_t* p = reinterpret_cast<uint8_t*>(const_cast<char*>(str.c_str()));
+
+    Encoding* e = Encoding::from_index(state, encodings[sym->index()]);
+    OnigEncodingType* enc = e->get_encoding();
+
+    // Constants start with A-Z, followed by alphanumeric characters or '_' or
+    // non-ascii character.
+    if(isupper(*p)) {
+      uint8_t* e = p + size;
+      int n = 0, code = 0;
+
+      for(++p; p < e; p += n) {
+        n = Encoding::precise_mbclen(p, e, enc);
+        if(!ONIGENC_MBCLEN_CHARFOUND_P(n)) {
+          return SymbolTable::eNormal;
+        }
+
+        n = ONIGENC_MBCLEN_CHARFOUND_LEN(n);
+        code = ONIGENC_MBC_TO_CODE(enc, p, p + n);
+        if(!(ONIGENC_IS_CODE_ALNUM(enc, code) || *p == '_' || !ISASCII(*p))) {
+          return SymbolTable::eNormal;
+        }
+      }
 
-    // Constants start with A-Z
-    if(isupper(one)) {
-      return SymbolTable::Constant;
+      return SymbolTable::eConstant;
     }
 
-    if(one == '@') {
+    if(p[0] == '@') {
       // A class variable begins with @@
-      if(size > 1 && str[1] == '@') {
-        return SymbolTable::CVar;
+      if(size > 1 && p[1] == '@') {
+        return SymbolTable::eCVar;
       }
 
       // An instance variable can't start with a digit and can't be just @.
-      if((size == 1) || (size > 1 && ISDIGIT(str[1]))) {
-        return SymbolTable::Normal;
+      if((size == 1) || (size > 1 && ISDIGIT(p[1]))) {
+        return SymbolTable::eNormal;
       }
 
       // An instance variable begins with @
-      return SymbolTable::IVar;
+      return SymbolTable::eIVar;
     }
 
     // A system variable begins with __
-    if(size > 2 && one == '_' && str[1] == '_') {
-      return SymbolTable::System;
+    if(size > 2 && p[0] == '_' && p[1] == '_') {
+      return SymbolTable::eSystem;
     }
 
     // Everything else is normal
-    return SymbolTable::Normal;
+    return SymbolTable::eNormal;
   }
 
   SymbolTable::Kind SymbolTable::kind(STATE, const Symbol* sym) {
     utilities::thread::SpinLock::LockGuard guard(lock_);
-    return kinds[sym->index()];
+
+    Kind k = kinds[sym->index()];
+
+    if(k == eUnknown) {
+      k = kinds[sym->index()] = detect_kind(state, sym);
+    }
+
+    return k;
   }
 
   size_t SymbolTable::add(std::string str, int enc) {
     bytes_used_ += (str.size() + sizeof(std::string) + sizeof(int) + sizeof(Kind));
 
     strings.push_back(str);
     encodings.push_back(enc);
-    kinds.push_back(detect_kind(str.data(), str.size()));
+    kinds.push_back(eUnknown);
     return strings.size() - 1;
   }
 

diff --git a/vm/symbol_table.hpp b/vm/symbol_table.hpp
@@ -45,11 +45,12 @@ namespace rubinius {
     // easily about it without having to get the string representation
     // and perform comparisons against the char data.
     enum Kind {
-      Normal,
-      Constant,
-      IVar,
-      CVar,
-      System
+      eUnknown,
+      eNormal,
+      eConstant,
+      eIVar,
+      eCVar,
+      eSystem
     };
 
     typedef std::vector<Kind> SymbolKinds;
@@ -62,8 +63,6 @@ namespace rubinius {
     utilities::thread::SpinLock lock_;
     size_t bytes_used_;
 
-    Symbol* lookup(const char* str, size_t length, int enc, uint32_t seed);
-
   public:
 
     SymbolTable()
@@ -79,6 +78,7 @@ namespace rubinius {
     Symbol* lookup(SharedState* shared, const std::string& str);
     Symbol* lookup(STATE, const std::string& str);
     Symbol* lookup(STATE, const char* str, size_t length);
+    Symbol* lookup(const char* str, size_t length, int enc, uint32_t seed);
     Symbol* lookup(STATE, String* str);
     String* lookup_string(STATE, const Symbol* sym);
 
@@ -91,7 +91,7 @@ namespace rubinius {
     Kind kind(STATE, const Symbol* sym);
 
     size_t add(std::string str, int enc);
-    static Kind detect_kind(const char* str, size_t size);
+    Kind detect_kind(STATE, const Symbol* sym);
   };
 };
 

diff --git a/vm/test/test_symbol_table.hpp b/vm/test/test_symbol_table.hpp
@@ -23,19 +23,17 @@ class TestSymbolTable : public CxxTest::TestSuite, public VMTest {
   }
 
   void test_detect_kind_with_constant() {
-    const char *input = "Pettson";
+    Symbol* sym = symbols->lookup(state, "Pettson");
+    SymbolTable::Kind kind = symbols->detect_kind(state, sym);
 
-    SymbolTable::Kind kind = symbols->detect_kind(input, strlen(input));
-
-    TS_ASSERT_EQUALS(kind, SymbolTable::Constant);
+    TS_ASSERT_EQUALS(kind, SymbolTable::eConstant);
   }
 
   void test_detect_kind_with_unicode_constant() {
-    const char *input = "Pettsonλ";
-
-    SymbolTable::Kind kind = symbols->detect_kind(input, strlen(input));
+    Symbol* sym = symbols->lookup("Pettsonλ", 9, Encoding::eUtf8, 0);
+    SymbolTable::Kind kind = symbols->detect_kind(state, sym);
 
-    TS_ASSERT_EQUALS(kind, SymbolTable::Constant);
+    TS_ASSERT_EQUALS(kind, SymbolTable::eConstant);
   }
 
   void test_lookup_with_c_str() {