Closed Bug 618422 Opened 11 years ago Closed 11 years ago

jstracer msvc warning in profiler code

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: sayrer, Assigned: billm)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

> jstracer.cpp(16915) : warning C4390: ';' : empty controlled statement found; 
> is this the intent?

16904             if (prof) {
16905                 /*
16906                  * Note that execOK for the inner loop is left unchanged. So even
16907                  * if we trace the inner loop, we will never call that trace
16908                  * on its own. We'll only call it from this trace.
16909                  */
16910                 prof->profiled = true;
16911                 prof->traceOK = true;
16912                 if (IsBlacklisted(loop.top))
16913                     debug_only_printf(LC_TMProfiler, "Unblacklisting at %d\n",
16914                                       js_PCToLineNumber(cx, loop.script, loop.top));
16915                 Unblacklist(loop.script, loop.top);
16916             }

What I want to believe is that an optimized build is being compiled this way:

> if (IsBlacklisted(loop.top)); // <-- C4930

and the line number is off by one. But I am still puzzled. Doesn't that mean the code is doing this:

>  if (IsBlacklisted(loop.top)) {
>    // compiled away
>  }
>
>  Unblacklist(loop.script, loop.top);

Shouldn't we unblacklist only those loops that are blacklisted?
Assignee: general → wmccloskey
blocking2.0: --- → betaN+
Attached patch fixSplinter Review
I'm going to blame this on the debug_only_printf macro, which I fixed. I also fixed it so that we only blacklist loops that are not already blacklisted, since that probably makes the code a little easier to read.
Attachment #497301 - Flags: review?(sayrer)
Comment on attachment 497301 [details] [diff] [review]
fix

This is an easy patch and a blocker. I just want to get it out of the way.
Attachment #497301 - Flags: review?(sayrer) → review?(dmandelin)
Attachment #497301 - Flags: review?(dmandelin) → review+
http://hg.mozilla.org/mozilla-central/rev/6545473b75b4
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.