Last Comment Bug 762182 - nsScriptLoader.cpp:398:12: warning: unused variable 'rootElement' [-Wunused-variable]
: nsScriptLoader.cpp:398:12: warning: unused variable 'rootElement' [-Wunused-v...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla16
Assigned To: Jignesh Kakadiya [:jhk]
:
:
Mentors:
Depends on:
Blocks: buildwarning 738380
  Show dependency treegraph
 
Reported: 2012-06-06 12:11 PDT by Daniel Holbert [:dholbert]
Modified: 2012-06-09 19:44 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.20 KB, patch)
2012-06-06 12:57 PDT, Jignesh Kakadiya [:jhk]
Ms2ger: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2012-06-06 12:11:45 PDT
{
../../../../mozilla/content/base/src/nsScriptLoader.cpp: In member function 'bool nsScriptLoader::ProcessScriptElement(nsIScriptElement*)':
../../../../mozilla/content/base/src/nsScriptLoader.cpp:398:12: warning: unused variable 'rootElement' [-Wunused-variable]
}

Variable was added (& used) here:
https://hg.mozilla.org/mozilla-central/diff/bc10dcdc3b1e/content/base/src/nsScriptLoader.cpp#l1.55

and then its only use was removed in bug 738380, here:
https://hg.mozilla.org/mozilla-central/diff/3eee3ceb400b/content/base/src/nsScriptLoader.cpp#l1.12
leaving us with the build warning.

So it looks like the variable can just be removed.
Comment 1 Daniel Holbert [:dholbert] 2012-06-06 12:24:57 PDT
...though the comment saying that we should rely on |rootElement| is still there.

So bug 738380 left that chunk of code with the comment & code disagreeing. Currently it looks like this:

> 395   // Default script language is whatever the root element specifies
> 396   // (which may come from a header or http-meta tag), or if there
> 397   // is no root element, from the script global object.
> 398   Element* rootElement = mDocument->GetRootElement();
> 399   PRUint32 typeID = nsIProgrammingLanguage::JAVASCRIPT;
https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsScriptLoader.cpp#395

Assuming the code is correct & rootElement is safe to remove, then we presumably want to remove that comment, too.
Comment 2 Jignesh Kakadiya [:jhk] 2012-06-06 12:57:08 PDT
Created attachment 630678 [details] [diff] [review]
Patch
Comment 3 Daniel Holbert [:dholbert] 2012-06-06 13:00:41 PDT
Comment on attachment 630678 [details] [diff] [review]
Patch

Based on bug 738380 comment 0, it sounds like the comment is obsolete & this patch is fine -- but Ms2ger or bz would know for sure.

Kicking review over to Ms2ger.
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2012-06-06 13:01:41 PDT
Comment on attachment 630678 [details] [diff] [review]
Patch

Yep, this is correct.
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-06-09 11:27:02 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/907796393e09
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-06-09 19:44:10 PDT
https://hg.mozilla.org/mozilla-central/rev/907796393e09

Note You need to log in before you can comment on or make changes to this bug.