LGTM On Wed, Sep 10, 2008 at 11:31 AM, <[EMAIL PROTECTED]> wrote: > I'd like you to do a code review. To review this change, run > > gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL > PROTECTED] > > Alternatively, to review the latest snapshot of this change > branch, run > > gvn --project https://v8.googlecode.com/svn review [EMAIL PROTECTED]/fix-57 > > to review the following change: > > [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-09-10 10:30:54 > +-100 (Wed, 10 Sep 2008) > > Description: > > Fixed bug #57. Introduced String::Utf8Value and replaced a bunch of > uses of String::AsciiValue with String::Utf8Value. > > > > > Affected Paths: > M //branches/bleeding_edge/include/v8.h > M //branches/bleeding_edge/samples/process.cc > M //branches/bleeding_edge/samples/shell.cc > M //branches/bleeding_edge/src/api.cc > M //branches/bleeding_edge/src/checks.cc > M //branches/bleeding_edge/src/messages.js > M //branches/bleeding_edge/src/runtime.js > A //branches/bleeding_edge/test/mjsunit/regress/regress-57.js > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: include/v8.h > =================================================================== > --- include/v8.h (^/branches/bleeding_edge/include/[EMAIL PROTECTED]) > +++ include/v8.h (^/changes/[EMAIL > PROTECTED]/fix-57/bleeding_edge/include/[EMAIL PROTECTED]) > @@ -880,6 +880,21 @@ class EXPORT String : public Primitive { > static Local<String> NewUndetectable(const uint16_t* data, int length = -1); > > /** > + * Converts an object to a utf8-encoded character array. Useful if > + * you want to print the object. > + */ > + class EXPORT Utf8Value { > + public: > + explicit Utf8Value(Handle<v8::Value> obj); > + ~Utf8Value(); > + char* operator*() { return str_; } > + int length() { return length_; } > + private: > + char* str_; > + int length_; > + }; > + > + /** > * Converts an object to an ascii string. > * Useful if you want to print the object. > */ > @@ -888,8 +903,10 @@ class EXPORT String : public Primitive { > explicit AsciiValue(Handle<v8::Value> obj); > ~AsciiValue(); > char* operator*() { return str_; } > + int length() { return length_; } > private: > char* str_; > + int length_; > }; > > /** > @@ -900,8 +917,10 @@ class EXPORT String : public Primitive { > explicit Value(Handle<v8::Value> obj); > ~Value(); > uint16_t* operator*() { return str_; } > + int length() { return length_; } > private: > uint16_t* str_; > + int length_; > }; > }; > > Index: samples/process.cc > =================================================================== > --- samples/process.cc (^/branches/bleeding_edge/samples/[EMAIL PROTECTED]) > +++ samples/process.cc (^/changes/[EMAIL > PROTECTED]/fix-57/bleeding_edge/samples/[EMAIL PROTECTED]) > @@ -137,7 +137,7 @@ static Handle<Value> LogCallback(const Arguments& > if (args.Length() < 1) return v8::Undefined(); > HandleScope scope; > Handle<Value> arg = args[0]; > - String::AsciiValue value(arg); > + String::Utf8Value value(arg); > HttpRequestProcessor::Log(*value); > return v8::Undefined(); > } > @@ -206,7 +206,7 @@ bool JsHttpRequestProcessor::ExecuteScript(Handle< > // Compile the script and check for errors. > Handle<Script> compiled_script = Script::Compile(script); > if (compiled_script.IsEmpty()) { > - String::AsciiValue error(try_catch.Exception()); > + String::Utf8Value error(try_catch.Exception()); > Log(*error); > // The script failed to compile; bail out. > return false; > @@ -216,7 +216,7 @@ bool JsHttpRequestProcessor::ExecuteScript(Handle< > Handle<Value> result = compiled_script->Run(); > if (result.IsEmpty()) { > // The TryCatch above is still in effect and will have caught the error. > - String::AsciiValue error(try_catch.Exception()); > + String::Utf8Value error(try_catch.Exception()); > Log(*error); > // Running the script failed; bail out. > return false; > @@ -262,7 +262,7 @@ bool JsHttpRequestProcessor::Process(HttpRequest* > Handle<Value> argv[argc] = { request_obj }; > Handle<Value> result = process_->Call(context_->Global(), argc, argv); > if (result.IsEmpty()) { > - String::AsciiValue error(try_catch.Exception()); > + String::Utf8Value error(try_catch.Exception()); > Log(*error); > return false; > } else { > @@ -332,8 +332,8 @@ map<string, string>* JsHttpRequestProcessor::Unwra > // Convert a JavaScript string to a std::string. To not bother too > // much with string encodings we just use ascii. > string ObjectToString(Local<Value> value) { > - String::AsciiValue ascii_value(value); > - return string(*ascii_value); > + String::Utf8Value utf8_value(value); > + return string(*utf8_value); > } > > > Index: samples/shell.cc > =================================================================== > --- samples/shell.cc (^/branches/bleeding_edge/samples/[EMAIL PROTECTED]) > +++ samples/shell.cc (^/changes/[EMAIL > PROTECTED]/fix-57/bleeding_edge/samples/[EMAIL PROTECTED]) > @@ -102,7 +102,7 @@ v8::Handle<v8::Value> Print(const v8::Arguments& a > } else { > printf(" "); > } > - v8::String::AsciiValue str(args[i]); > + v8::String::Utf8Value str(args[i]); > printf("%s", *str); > } > printf("\n"); > @@ -116,7 +116,7 @@ v8::Handle<v8::Value> Print(const v8::Arguments& a > v8::Handle<v8::Value> Load(const v8::Arguments& args) { > for (int i = 0; i < args.Length(); i++) { > v8::HandleScope handle_scope; > - v8::String::AsciiValue file(args[i]); > + v8::String::Utf8Value file(args[i]); > v8::Handle<v8::String> source = ReadFile(*file); > if (source.IsEmpty()) { > return v8::ThrowException(v8::String::New("Error loading file")); > @@ -202,7 +202,7 @@ bool ExecuteString(v8::Handle<v8::String> source, > if (print_result && !result->IsUndefined()) { > // If all went well and the result wasn't undefined then print > // the returned value. > - v8::String::AsciiValue str(result); > + v8::String::Utf8Value str(result); > printf("%s\n", *str); > } > return true; > @@ -213,7 +213,7 @@ bool ExecuteString(v8::Handle<v8::String> source, > > void ReportException(v8::TryCatch* try_catch) { > v8::HandleScope handle_scope; > - v8::String::AsciiValue exception(try_catch->Exception()); > + v8::String::Utf8Value exception(try_catch->Exception()); > v8::Handle<v8::Message> message = try_catch->Message(); > if (message.IsEmpty()) { > // V8 didn't provide any extra information about this error; just > @@ -221,11 +221,11 @@ void ReportException(v8::TryCatch* try_catch) { > printf("%s\n", *exception); > } else { > // Print (filename):(line number): (message). > - v8::String::AsciiValue filename(message->GetScriptResourceName()); > + v8::String::Utf8Value filename(message->GetScriptResourceName()); > int linenum = message->GetLineNumber(); > printf("%s:%i: %s\n", *filename, linenum, *exception); > // Print line of source code. > - v8::String::AsciiValue sourceline(message->GetSourceLine()); > + v8::String::Utf8Value sourceline(message->GetSourceLine()); > printf("%s\n", *sourceline); > // Print wavy underline (GetUnderline is deprecated). > int start = message->GetStartColumn(); > @@ -237,5 +237,10 @@ void ReportException(v8::TryCatch* try_catch) { > printf("^"); > } > printf("\n"); > + v8::Handle<v8::String> stack_trace = message->GetStackTrace(); > + if (!stack_trace.IsEmpty()) { > + v8::String::Utf8Value stack_trace_str(stack_trace); > + printf("%s\n", *stack_trace_str); > + } > } > } > Index: src/api.cc > =================================================================== > --- src/api.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/api.cc (^/changes/[EMAIL PROTECTED]/fix-57/bleeding_edge/src/[EMAIL > PROTECTED]) > @@ -2629,13 +2629,40 @@ void V8::SetGlobalGCEpilogueCallback(GCCallback ca > } > > > +String::Utf8Value::Utf8Value(v8::Handle<v8::Value> obj) { > + EnsureInitialized("v8::String::Utf8Value::Utf8Value()"); > + HandleScope scope; > + TryCatch try_catch; > + Handle<String> str = obj->ToString(); > + if (str.IsEmpty()) { > + str_ = NULL; > + length_ = 0; > + } else { > + length_ = str->Utf8Length(); > + str_ = i::NewArray<char>(length_ + 1); > + str->WriteUtf8(str_); > + } > +} > + > + > +String::Utf8Value::~Utf8Value() { > + i::DeleteArray(str_); > +} > + > + > String::AsciiValue::AsciiValue(v8::Handle<v8::Value> obj) { > EnsureInitialized("v8::String::AsciiValue::AsciiValue()"); > HandleScope scope; > + TryCatch try_catch; > Handle<String> str = obj->ToString(); > - int length = str->Length(); > - str_ = i::NewArray<char>(length + 1); > - str->WriteAscii(str_); > + if (str.IsEmpty()) { > + str_ = NULL; > + length_ = 0; > + } else { > + length_ = str->Length(); > + str_ = i::NewArray<char>(length_ + 1); > + str->WriteAscii(str_); > + } > } > > > @@ -2647,10 +2674,16 @@ String::AsciiValue::~AsciiValue() { > String::Value::Value(v8::Handle<v8::Value> obj) { > EnsureInitialized("v8::String::Value::Value()"); > HandleScope scope; > + TryCatch try_catch; > Handle<String> str = obj->ToString(); > - int length = str->Length(); > - str_ = i::NewArray<uint16_t>(length + 1); > - str->Write(str_); > + if (str.IsEmpty()) { > + str_ = NULL; > + length_ = 0; > + } else { > + length_ = str->Length(); > + str_ = i::NewArray<uint16_t>(length_ + 1); > + str->Write(str_); > + } > } > > > Index: src/checks.cc > =================================================================== > --- src/checks.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/checks.cc (^/changes/[EMAIL > PROTECTED]/fix-57/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -74,8 +74,8 @@ void CheckEqualsHelper(const char* file, > const char* value_source, > v8::Handle<v8::Value> value) { > if (!expected->Equals(value)) { > - v8::String::AsciiValue value_str(value); > - v8::String::AsciiValue expected_str(expected); > + v8::String::Utf8Value value_str(value); > + v8::String::Utf8Value expected_str(expected); > V8_Fatal(file, line, > "CHECK_EQ(%s, %s) failed\n# Expected: %s\n# Found: %s", > expected_source, value_source, *expected_str, *value_str); > @@ -90,7 +90,7 @@ void CheckNonEqualsHelper(const char* file, > const char* value_source, > v8::Handle<v8::Value> value) { > if (unexpected->Equals(value)) { > - v8::String::AsciiValue value_str(value); > + v8::String::Utf8Value value_str(value); > V8_Fatal(file, line, "CHECK_NE(%s, %s) failed\n# Value: %s", > unexpected_source, value_source, *value_str); > } > Index: src/messages.js > =================================================================== > --- src/messages.js (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/messages.js (^/changes/[EMAIL > PROTECTED]/fix-57/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -148,6 +148,8 @@ function MakeGenericError(constructor, type, args) > args[i] = elem.slice(0,20).concat("..."); > } > } > + } else if (IS_UNDEFINED(args)) { > + args = []; > } > > var e = new constructor(); > Index: src/runtime.js > =================================================================== > --- src/runtime.js (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/runtime.js (^/changes/[EMAIL > PROTECTED]/fix-57/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -439,7 +439,7 @@ function ToObject(x) { > if (IS_STRING(x)) return new $String(x); > if (IS_NUMBER(x)) return new $Number(x); > if (IS_BOOLEAN(x)) return new $Boolean(x); > - if (x == null) throw %MakeTypeError('null_to_object'); > + if (x == null) throw %MakeTypeError('null_to_object', []); > return x; > }; > > Index: test/mjsunit/regress/regress-57.js > =================================================================== > --- test/mjsunit/regress/regress-57.js (added) > +++ test/mjsunit/regress/regress-57.js (^/changes/[EMAIL > PROTECTED]/fix-57/bleeding_edge/test/mjsunit/regress/[EMAIL PROTECTED]) > @@ -0,0 +1,5 @@ > +try { > + delete (void 0).x; > +} catch (e) { > + print(e.toString()); > +} > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
