Assertion failure when using --log-regexp

src/log.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_LOG_H_
 #define V8_LOG_H_
 
 #include <string>
 
 #include "src/allocation.h"
 #include "src/base/platform/elapsed-timer.h"
 #include "src/base/platform/platform.h"
+#include "src/flags.h"

[Jakob Kummerow, 2015/06/24 13:40:01]

I don't think flags.h is needed below.

[Djordje.Pesic, 2015/06/25 08:06:16]

I have to include flags.h because log-utils.h uses flags defined in flags.h.
Alternatively, I can include flags.h in log-utils.h (log-utils.h doesn't include
it). Objects.h includes flags.h, but presubmit script complains about
alphabetical order of includes.Also, presubmit script doesn't allow forward
declarations like Log::MessageBuilder.

[Jakob Kummerow, 2015/06/29 09:11:50]

Yes, please include it exactly where it's needed (so log-utils.h in this case). 

As a general rule, every file should include what it needs, no more, no less.
V8's includes are a big mess and rather far from following this rule everywhere,
but whenever we make changes we should strive for incremental improvement,
rather than making the situation even worse.

[Djordje.Pesic, 2015/06/29 13:08:38]

Done.

+#include "src/log-utils.h"
 #include "src/objects.h"
 
 namespace v8 {
 
 namespace base {
 class Semaphore;
 }
 
 namespace internal {
 
@@ skipping 325 matching lines @@
 
   // Emits callback event messages.
   void CallbackEventInternal(const char* prefix,
                              Name* name,
                              Address entry_point);
 
   // Internal configurable move event.
   void MoveEventInternal(LogEventsAndTags event, Address from, Address to);
 
   // Emits the source code of a regexp. Used by regexp events.
-  void LogRegExpSource(Handle<JSRegExp> regexp);
-
+  void LogRegExpSource(Handle<JSRegExp> regexp, Log::MessageBuilder& msg);

[Jakob Kummerow, 2015/06/24 13:40:01]

The style guide forbids non-const references. Either use a const reference, or
use a pointer, which would have the additional benefit that you can
forward-declare Log::MessageBuilder (and drop the log-utils.h include).

[Djordje.Pesic, 2015/06/25 08:06:16]

I have to include log-utils.h because class MessageBuilder is defined in class
Log, so forward-declaration of Log::MessageBuilder doesn't compile without
log-utils.h included. So, I used const reference.

[Jakob Kummerow, 2015/06/29 09:11:50]

Hm... But I'd really like to get as far away as possible from "every header
includes every other header".

As far as I can see, LogRegExpSource only has a single call site in
RegExpCompileEvent, and only uses the logger to get the isolate. So you could
move it out of the Logger class and into an anonymous namespace, and
additionally pass in the isolate, to avoid the additional include (log.cc
includes log-utils.h anyway).
OK.

[Djordje.Pesic, 2015/06/29 13:08:38]

Done.

   // Used for logging stubs found in the snapshot.
   void LogCodeObject(Object* code_object);
 
   // Helper method. It resets name_buffer_ and add tag name into it.
   void InitNameBuffer(LogEventsAndTags tag);
 
   // Emits a profiler tick event. Used by the profiler thread.
   void TickEvent(TickSample* sample, bool overflow);
 
   void ApiEvent(const char* name, ...);
@@ skipping 156 matching lines @@
                                  int length) = 0;
 
   NameBuffer* name_buffer_;
 };
 
 
 } }  // namespace v8::internal
 
 
 #endif  // V8_LOG_H_