You need to reconfigure your editor to avoid tabs. We don't allow tabs in V8.
On Thu, Sep 11, 2008 at 10:48 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/branches/bleeding_edge > lrn/[EMAIL PROTECTED] > > Alternatively, to review the latest snapshot of this change > branch, run > > gvn --project https://v8.googlecode.com/svn/branches/bleeding_edge review > lrn/lrn_relog > > to review the following change: > > *lrn/[EMAIL PROTECTED] | lrn | 2008-09-11 09:45:22 +-100 (Thu, 11 Sep 2008) > > Description: > > Added -log-regexp option to log all compilations and executions of regular > expressions. > Slightly modified SmartPointer. > Made String.ToWideCString return a SmartPointer instead of a plain pointer. > > > > > Affected Paths: > M //branches/bleeding_edge/src/jsregexp.cc > M //branches/bleeding_edge/src/log.cc > M //branches/bleeding_edge/src/log.h > M //branches/bleeding_edge/src/messages.cc > M //branches/bleeding_edge/src/objects.cc > M //branches/bleeding_edge/src/objects.h > M //branches/bleeding_edge/src/smart-pointer.h > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: src/jsregexp.cc > =================================================================== > --- src/jsregexp.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > > +++ src/jsregexp.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL > PROTECTED]) > > @@ -205,6 +205,8 @@ Handle<Object> RegExpImpl::JsreCompile(Handle<JSVa > > value->set(INTERNAL_INDEX, *internal); > re->set_value(*value); > > + LOG(RegExpCompileEvent(re)); > + > return re; > } > > @@ -223,6 +225,8 @@ Handle<Object> RegExpImpl::JsreExecOnce(Handle<JSV > > const JSRegExp* js_regexp = > reinterpret_cast<JSRegExp*>(internal->GetDataStartAddress()); > > + LOG(RegExpExecEvent(regexp, previous_index, subject)); > + > rc = jsRegExpExecute(js_regexp, two_byte_subject, > subject->length(), > previous_index, > Index: src/log.cc > =================================================================== > --- src/log.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > > +++ src/log.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) > > @@ -52,6 +52,8 @@ DEFINE_bool(log_state_changes, false, "Log state c > > DEFINE_bool(log_suspect, false, "Log suspect operations."); > DEFINE_bool(prof, false, > "Log statistical profiling information (implies --log-code)."); > +DEFINE_bool(log_regexp, false, > + "Log regular expression execution."); > DEFINE_bool(sliding_state_window, false, > "Update sliding state window counters."); > > @@ -368,6 +370,76 @@ void Logger::SharedLibraryEvent(const wchar_t* lib > > #endif > } > > +void Logger::LogRegExpSource(const Handle<JSValue> val) { > + // Prints "/" + re.source + "/" + > + // (re.global?"g":"") + (re.ignorecase?"i":"") + > (re.multiline?"m":"") > + > + Handle<Object> source = GetProperty(val, "source"); > + if (!source->IsString()) { > + fprintf(logfile_, "no source"); > + return; > + } > + Handle<String> sourceString = Handle<String>::cast(source); > + > + SmartPointer<uc16> cstringAlloc = sourceString->ToWideCString(); > + fprintf(logfile_, "/"); > + for(size_t i = 0, n = sourceString->length(); i < n; i++) { > + uc16 c = cstringAlloc[i]; > + if (c < 32 || (c > 126 && c <= 255)) { > + fprintf(logfile_, "\\x%02x", c); > + } else if (c > 255) { > + fprintf(logfile_, "\\u%04x", c); > + } else { > + fprintf(logfile_, "%lc", c); > + } > + } > + fprintf(logfile_, "/"); > + > + // global flag > + Handle<Object> global = GetProperty(val, "global"); > + if (global->IsTrue()) { > + fprintf(logfile_, "g"); > + } > + // ignorecase flag > + Handle<Object> ignorecase = GetProperty(val, "ignoreCase"); > + if (ignorecase->IsTrue()) { > + fprintf(logfile_, "i"); > + } > + // multiline flag > + Handle<Object> multiline = GetProperty(val, "multiline"); > + if (multiline->IsTrue()) { > + fprintf(logfile_, "m"); > + } > +} > + > + > +void Logger::RegExpCompileEvent(const Handle<JSValue> pattern) { > +#ifdef ENABLE_LOGGING_AND_PROFILING > + if (logfile_ == NULL || !FLAG_log_regexp) { return; } > + ScopedLock sl(mutex_); > + > + fprintf(logfile_, "regexp-compile,"); > + LogRegExpSource(pattern); > + fprintf(logfile_, "\n"); > + > +#endif > +} > + > + > +void Logger::RegExpExecEvent(const Handle<JSValue> val, > + int start_index, > + Handle<String> > string) { > +#ifdef ENABLE_LOGGING_AND_PROFILING > + if (logfile_ == NULL || !FLAG_log_regexp) { return; } > + ScopedLock sl(mutex_); > + > + fprintf(logfile_, "regexp-run,"); > + LogRegExpSource(val); > + fprintf(logfile_, ",0x%08x,%d..%d\n", string->Hash(), start_index, > string->length()); > +#endif > +} > + > + > void Logger::ApiIndexedSecurityCheck(uint32_t index) { > #ifdef ENABLE_LOGGING_AND_PROFILING > if (logfile_ == NULL || !FLAG_log_api) return; > @@ -612,6 +684,7 @@ bool Logger::Setup() { > > FLAG_log_gc = true; > FLAG_log_suspect = true; > FLAG_log_handles = true; > + FLAG_log_regexp = true; > } > > // --prof implies --log-code. > @@ -620,7 +693,7 @@ bool Logger::Setup() { > > // Each of the individual log flags implies --log. Check after > // checking --log-all and --prof in case they set --log-code. > if (FLAG_log_api || FLAG_log_code || FLAG_log_gc || > - FLAG_log_handles || FLAG_log_suspect) { > + FLAG_log_handles || FLAG_log_suspect || FLAG_log_regexp) { > FLAG_log = true; > } > > Index: src/log.h > =================================================================== > --- src/log.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > > +++ src/log.h (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) > > @@ -174,6 +174,14 @@ class Logger { > > unsigned start, > unsigned end); > > + // ==== Events logged by --log-regexp ==== > + // Regexp compilation and execution events. > + > + static void RegExpCompileEvent(const Handle<JSValue> regexp); > + > + static void RegExpExecEvent( > + const Handle<JSValue> regexp, int start_index, Handle<String> > string); > + > #ifdef ENABLE_LOGGING_AND_PROFILING > static StateTag state() { > return current_state_ ? current_state_->state() : OTHER; > @@ -182,6 +190,10 @@ class Logger { > > > #ifdef ENABLE_LOGGING_AND_PROFILING > private: > + > + // Emits the source code of a regexp. Used by regexp events. > + static void Logger::LogRegExpSource(const Handle<JSValue> regexp); > + > // Emits a profiler tick event. Used by the profiler thread. > static void TickEvent(TickSample* sample, bool overflow); > > Index: src/messages.cc > =================================================================== > --- src/messages.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > > +++ src/messages.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL > PROTECTED]) > > @@ -46,7 +46,7 @@ void MessageHandler::DefaultMessageReport(const Me > > } else { > HandleScope scope; > Handle<Object> data(loc->script()->name()); > - SmartPointer<char> data_str = NULL; > + SmartPointer<char> data_str; > if (data->IsString()) > data_str = Handle<String>::cast(data)->ToCString(DISALLOW_NULLS); > PrintF("%s:%i: %s\n", *data_str ? *data_str : "<unknown>", > Index: src/objects.cc > =================================================================== > --- src/objects.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > > +++ src/objects.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL > PROTECTED]) > > @@ -3023,11 +3023,11 @@ const uc16* String::GetTwoByteData(unsigned start) > > } > > > -uc16* String::ToWideCString(RobustnessFlag robust_flag) { > +SmartPointer<uc16> String::ToWideCString(RobustnessFlag robust_flag) { > ASSERT(NativeAllocationChecker::allocation_allowed()); > > if (robust_flag == ROBUST_STRING_TRAVERSAL && !LooksValid()) { > - return NULL; > + return SmartPointer<uc16>(); > } > > Access<StringInputBuffer> buffer(&string_input_buffer); > @@ -3041,7 +3041,7 @@ const uc16* String::GetTwoByteData(unsigned start) > > result[i++] = character; > } > result[i] = 0; > - return result; > + return SmartPointer<uc16>(result); > } > > > Index: src/objects.h > =================================================================== > --- src/objects.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > > +++ src/objects.h (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL > PROTECTED]) > > @@ -2864,7 +2864,7 @@ class String: public HeapObject { > > // ROBUST_STRING_TRAVERSAL invokes behaviour that is robust This means it > // handles unexpected data without causing assert failures and it does not > // do any heap allocations. This is useful when printing stack traces. > - uc16* ToWideCString(RobustnessFlag robustness_flag = > FAST_STRING_TRAVERSAL); > + SmartPointer<uc16> ToWideCString(RobustnessFlag robustness_flag = > FAST_STRING_TRAVERSAL); > > // Tells whether the hash code has been computed. > inline bool HasHashCode(); > Index: src/smart-pointer.h > =================================================================== > --- src/smart-pointer.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > > +++ src/smart-pointer.h (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL > PROTECTED]) > > @@ -31,21 +31,20 @@ > > namespace v8 { namespace internal { > > > -// A 'scoped pointer' that calls delete[] on its pointer when the > +// A 'scoped array pointer' that calls DeleteArray on its pointer when the > // destructor is called. > template<typename T> > class SmartPointer { > public: > - // Construct a scoped pointer from a plain one. > - inline SmartPointer(T* pointer) : p(pointer) {} > > + // Default constructor. Construct an empty scoped pointer. > + inline SmartPointer() : p(NULL) {} > > - // When the destructor of the scoped pointer is executed the plain pointer > - // is deleted using DeleteArray. This implies that you must allocate with > - // NewArray. > - inline ~SmartPointer() { if (p) DeleteArray(p); } > > + // Construct a scoped pointer from a plain one. > + explicit inline SmartPointer(T* pointer) : p(pointer) {} > > + > // Copy constructor removes the pointer from the original to avoid double > // freeing. > inline SmartPointer(const SmartPointer<T>& rhs) : p(rhs.p) { > @@ -53,14 +52,21 @@ class SmartPointer { > > } > > > + // When the destructor of the scoped pointer is executed the plain pointer > + // is deleted using DeleteArray. This implies that you must allocate with > + // NewArray. > + inline ~SmartPointer() { if (p) DeleteArray(p); } > + > + > // You can get the underlying pointer out with the * operator. > inline T* operator*() { return p; } > > > - // You can use -> as if it was a plain pointer. > - inline T* operator->() { return p; } > + // You can use [n] to index as if it was a plain pointer > + inline T& operator[](size_t i) { > + return p[i]; > + } > > - > // We don't have implicit conversion to a T* since that hinders migration: > // You would not be able to change a method from returning a T* to > // returning an SmartPointer<T> and then get errors wherever it is used. > @@ -81,9 +87,10 @@ class SmartPointer { > > // double freeing. > inline SmartPointer& operator=(const SmartPointer<T>& rhs) { > ASSERT(p == NULL); > - p = rhs.p; > - const_cast<SmartPointer<T>&>(rhs).p = NULL; > - return *this; > + T* tmp = rhs.p; // swap to handle self-assignment > + const_cast<SmartPointer<T>&>(rhs).p = NULL; > + p = tmp; > + return *this; > } > > > > > > > -- Erik Corry, Software Engineer Google Denmark ApS. CVR nr. 28 86 69 84 c/o Philip & Partners, 7 Vognmagergade, P.O. Box 2227, DK-1018 Copenhagen K, Denmark. --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
