Last Comment Bug 537870 - VC7.1 confused by initialized declaration in for-statement conditional
: VC7.1 confused by initialized declaration in for-statement conditional
Status: RESOLVED FIXED
[fixed-lorentz]
:
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Karl Tomlinson (:karlt)
:
Mentors:
Depends on:
Blocks: 518506
  Show dependency treegraph
 
Reported: 2010-01-04 20:34 PST by Karl Tomlinson (:karlt)
Modified: 2010-04-07 06:11 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.4-fixed


Attachments
xpwidget patch (1.36 KB, patch)
2010-01-06 15:57 PST, Karl Tomlinson (:karlt)
roc: review+
Details | Diff | Splinter Review

Description Karl Tomlinson (:karlt) 2010-01-04 20:34:44 PST
Bug 518506 comment 43 pointed out that an initialized declaration in the
conditional of a for statement, added in that bug, confused VC7.1.
http://hg.mozilla.org/mozilla-central/annotate/5a63989cada8/widget/src/xpwidgets/nsBaseWidget.cpp#l1005

AFAIK this is valid ISO C++, since at least 1998, but
https://developer.mozilla.org/En/Developer_Guide/Build_Instructions/Windows_Prerequisites
says that mozilla-central and mozilla-1.9.2 (should) build with VC7.1.

(We don't support jemalloc with VC7.1, but that and VC8 may be the reason why
jemalloc is off by default with msvc.
http://hg.mozilla.org/mozilla-central/annotate/5a63989cada8//configure.in#l6713
)

Neil, do you know whether this is the only problem when compiling with VC7.1?
I'm assuming this caused a compiler error.
Do you know whether there is a good way to un-confuse VC7.1?
I guess the only option is to move the declaration outside the loop?
(Out of interest, I guess the same problem would happen with
if, while, and switch statements?)
Comment 1 neil@parkwaycc.co.uk 2010-01-05 02:12:38 PST
(In reply to comment #0)
> Neil, do you know whether this is the only problem when compiling with VC7.1?
Actually some of tracemonkey doesn't compile with VC7.1, although there is a pending review (for some time now, so probably bitrotted...) I can't remember any other problems offhand.

> Do you know whether there is a good way to un-confuse VC7.1?
> I guess the only option is to move the declaration outside the loop?
That's what I did to get it to compile locally.

> (Out of interest, I guess the same problem would happen with
> if, while, and switch statements?)
Probably.
Comment 2 Karl Tomlinson (:karlt) 2010-01-06 15:57:20 PST
Apparently VC7.1 understands initialized declarations in while-loop
conditionals
http://mxr.mozilla.org/mozilla-central/search?string=while+%28&find=&findi=&filter=while+\%28[*\w]%2B+\w%2B+%3D+&hitlimit=&tree=mozilla-central

and maybe in if-statements
http://mxr.mozilla.org/mozilla-central/search?string=if+%28&find=&findi=&filter=if+\%28[*\w]%2B+\w%2B+%3D+&hitlimit=&tree=mozilla-central
e.g. http://hg.mozilla.org/mozilla-central/annotate/106fa9dc5896/testing/mochitest/ssltunnel/ssltunnel.cpp#l730

(I don't see any in switch-statements.)

BTW, there is a similar initialized declaration in a for-loop conditional
introduced after Neil's patches in bug 513909
http://hg.mozilla.org/mozilla-central/rev/ca95f2397bc5#l1.15
Comment 3 Karl Tomlinson (:karlt) 2010-01-06 15:57:30 PST
Created attachment 420442 [details] [diff] [review]
xpwidget patch

This is easy to fix, so let's fix.  Other options are declaring otherRect
(uninitialized) outside the for-loop, or declaring nextLink (initialized)
outside a while-loop.  But I like narrow variable scopes, so this dereferences
twice (when not optimized) to declare otherRect inside the for-loop.
Comment 4 Karl Tomlinson (:karlt) 2010-02-01 22:58:37 PST
http://hg.mozilla.org/mozilla-central/rev/29d0fadf3a20
Comment 5 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-03-25 07:42:42 PDT
http://hg.mozilla.org/projects/firefox-lorentz/rev/bdb92894e4d1
Comment 6 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-06 16:09:45 PDT
Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed
Comment 7 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-04-07 06:11:06 PDT
Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430

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