Convert #ifdef DEBUG variables into DebugOnly<T>

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

Mechanical rewrite of

#ifdef DEBUG
  T foo =
#endif
  Bar();

to

  DebugOnly<T> foo = Bar();
yeeehaaaw
$ find -type f -name 'js*' -exec mgrep -l '#ifdef DEBUG[^\n]*\n\s*\S+\s+\S+\s*=' {} \; | wc -l
27
$ find -type f -name 'js*' -exec mgrep -l '#ifdef DEBUG[^\n]*\n\s*\S+\s+\S+\s*=' {} \;
./jsstrinlines.h
./jsfun.h
./jsscan.cpp
./jsobj.cpp
./jsinterp.cpp
./jsapi.h
./jscntxtinlines.h
./jsscript.cpp
./jstracer.cpp
./jsatom.cpp
./jscntxt.h
./jsstr.cpp
./jsapi.cpp
./jspropertytree.cpp
./jscntxt.cpp
./jsgc.cpp
./jsscope.cpp
./shell/js.cpp
./jsemit.cpp
./jsarray.cpp
./jspropertycache.cpp
./jsobjinlines.h
./jsparse.cpp
./jsinterpinlines.h
./jsxml.cpp
./jstl.h
./jsfun.cpp
By which I meant

$ find -type f -name 'js*' -exec mgrep -l '\s*#ifdef DEBUG\s*\n\s*\S+\s+\S+\s*=\s*\n\s*#endif' {} \; 
./jsobj.cpp
./jsscript.cpp
./jstracer.cpp
./jscntxt.h
./jscntxt.cpp
./shell/js.cpp
./jsemit.cpp
I ended settling on

$ find -type f -name 'js*' -exec mgrep -l '#ifdef DEBUG[^\n]*\n\s*\S+\s+\S[^=]*=\s*\n\s*#endif' {} \; 

The files I left unfixed in js/src were in xpconnect and the following
./methodjit/FastOps.cpp
./methodjit/StubCalls.cpp
./yarr/yarr/RegexCompiler.cpp
Assignee: general → jones.chris.g
Attachment #523499 - Flags: review?(luke)
Sorry for the delay.  We could decrease the syntactic distinction between mozilla/mfbt and js/mfbt with:

// js/src/jsutil.h
#include "mozilla/Util.h"
namespace js { using namespace mozilla; }

Then, we could use "js::DebugOnly<T>" in headers and "DebugOnly<T>" in .cpps (where we already have 'using namespace js').

Is that reasonable or too weird?
No hurry.

Suits me, except for potential name collisions (bug 642381 comment 11).  I'd be fine dealing with that if/when it arises though.
(In reply to comment #7)
> Suits me, except for potential name collisions (bug 642381 comment 11).  I'd be
> fine dealing with that if/when it arises though.

Ah, I missed that comment before.  Since I'm thinking that js/mfbt is going to be js::Hash{Set,Map}, js::Vector and not much else, I'm not too worried; we should be able to keep them coherent.  (Btw, bug 632137 is indeed making its way to the front of my work queue :)
Attachment #523499 - Attachment is obsolete: true
Attachment #523499 - Flags: review?(luke)
Attachment #524472 - Flags: review?(luke)
Comment on attachment 524472 [details] [diff] [review]
Sprinkle some DebugOnly in js/src

Rock on
Attachment #524472 - Flags: review?(luke) → review+
Whiteboard: fixed-in-tracemonkey
cjones, 'tis customary to post the tracemonkey changeset URI when you mark as fixed-in-tracemonkey.

http://hg.mozilla.org/mozilla-central/rev/fa2c397985a2
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.