569686 - nativegen.py output should be stable

Status: VERIFIED FIXED

(Tamarin Graveyard :: Tools, defect, P4)

Description

[Edwin Smith]

occasionally we'll see build warnings that appear to come from unstable iteration order in either nativegen.py or asc, for example below.  If we can fix this instability in nativegen.py then it will suppress spurious build warnings and hg diff's, and eliminate a minor annoyance.

I beleive we also have build scripts that auto generate stubs, and revert (and re-touch) the file if there are no changes, to avoid full rebuilds.  making nativegen output stable will aid that use case.

http://tamarin-builds.mozilla.org/tamarin-redux/builders/mac-intel-10.5-compile/builds/202/steps/Compile_builtin.abc/logs/stdio

diff -r 509efc64f316 core/builtin.cpp
--- a/core/builtin.cpp	Wed Jun 02 12:53:35 2010 -0700
+++ b/core/builtin.cpp	Wed Jun 02 16:16:49 2010 -0400
@@ -4157,11 +4157,11 @@
 // Date_AS3_getMilliseconds
 // Date_AS3_getUTCDay
 // Date_AS3_getUTCHours
-// Number_private__minValue
+// Date_AS3_getUTCMilliseconds
 // Date_AS3_getTime
 // Date_AS3_getDay
 // __AS3___vec_Vector_double_AS3_pop
-// Date_AS3_getUTCMilliseconds
+// Number_private__minValue
 // Date_AS3_getMonth
 // Math_random
 // Date_AS3_getDate

Comment 1

[Edwin Smith]

I have no proof that nativegen.py is at fault; it could also be asc. 

Lars wrote:

... it would appear that there are some bugs in the script that rebuilds those files.  This is the diff that was produced here:

diff --git a/shell/shell_toplevel.h b/shell/shell_toplevel.h
--- a/shell/shell_toplevel.h
+++ b/shell/shell_toplevel.h
@@ -63,7 +63,7 @@
    class NewObjectSampleObject; //flash.sampler::NewObjectSample
    class SampleClass; //flash.sampler::Sample$
    class SampleObject; //flash.sampler::Sample
-    class ScriptObject; //avmplus::File
+    class ScriptObject; //flash.trace::Trace
    class StackFrameClass; //flash.sampler::StackFrame$
    class StackFrameObject; //flash.sampler::StackFrame
    class String; //String

That change is spurious, and wrong besides.  (I mean, it's just a comment, and the old comment was wrong too, so it's no worse than before, but it's a random change that just shouldn't be there.) ...

Comment 2

[Brent Baker]

Attachment: stabilize the order of comments

Patch includes regenerated code, but meat of the patch (all 3 lines) is in utils/nativegen.py

Ran this through the sandbox and all builders generated the exact same cpp/h files

Comment 3

[Trevor Baker]

magic!

Comment 4

[Edwin Smith]

The output looks stable, but I think nativegen.py still has a bug when multiple AS3 class types or instance types use the same underlying C++ class.  For example

-    class ScriptObject; //avmplus::File
+    class ScriptObject; //flash.trace::Trace

What AS3 name we put in the comment is arbitrary (before) or deterministic (after patch).  There are hundreds of AS3 types that map to ScriptObject.  We can make the output stable by always printing the first one.  Better if we improve the comment generation by listing them all.  Or, print one forward def per AS3 type, even if that means duplicate forward decls.

Scope creep? new patch on this bug? new bug?  comments welcome.

Comment 5

[Steven Johnson]

(In reply to comment #4)
> Better if we improve the comment generation by listing them all. 

My vote. New patch on this bug. Should be simple.

Comment 6

[Steven Johnson]

Attachment: Further sort output

Further cleanup per Edwin's suggestion

Comment 7

[Brent Baker]

Comment on attachment 481280 [details] [diff] [review]
Further sort output

Looks good, did a quick look at the generated diff and now ScriptObject and ObjectVectorObject are declared multiple times in the header, showing which classes are using it.

Comment 8

[Steven Johnson]

Why don't you land both patches at the same time.

Comment 9

[Brent Baker]

Patches pushed as http://hg.mozilla.org/tamarin-redux/rev/b6d76a45d414

Comment 10

[Brent Baker]

Build 5337:b6d76a45d414 has compiled the builtins on all platforms and there was no diff on any platform, all machines generated the same cpp/h files