module constant values

Fri Dec 16 03:55:00 GMT 2011

On 12/15/2011 04:14 PM, Jamison Hope wrote:
>
> near the top of BindingInitializer#emit(), but it's not clear to me
> whether that's the right place to make a change. Perhaps the
> BindingInitializer shouldn't be created in the first place if the
> Declaration's field has had a ConstantValue set. But this happens in
> SetExp#compile(), which has
>
> if (decl.shouldEarlyInit())
> BindingInitializer.create(decl, new_value, comp);
>
> and Declaration#shouldEarlyInit() seems written specifically so that
> for constants it will return true.

IIRC the concept of EARLY_INIT is declarations that should be initialized
"early" - i.e. at init-time (or clinit-time) before the <body> is evaluated
(normally, during "run").  This allows some "natural" forward referencing.

If a field has a ConstantValue attribute, then it is initialized
"super-early", which satisfies the goals of EARLY_INIT - and there is
no need for a duplicate initialization.

So I'm inclined to think the best fix is to not create the 
BindingInitializer
in the first case (less work that way), perhaps changing SetExp somewhat 
like:

     else if ((decl.shouldEarlyInit() || decl.isAlias())
         && decl.context instanceof ModuleExp
	&& isDefining() && ! decl.ignorable())
       {
         if (decl.shouldEarlyInit()
             && Attribute.get(decl.field, "ConstantValue") == null)
           BindingInitializer.create(decl, new_value, comp);
         if (needValue)
           {
             decl.load(this, 0, comp, Target.pushObject);
	    valuePushed = true;
	  }
       }

Maybe it would be cleaner to add to ConstantValueAttr a method:

   public static ConstantValueAttr get(AttrContainer container) {
       return Attribute.get(container, "ConstantValue");
   }

It's possible the above may not work - perhaps the ConstantValueAttr
is created too late.  It seems like it should be ok, but perhaps you
can look into it?
-- 
	--Per Bothner
per@bothner.com   http://per.bothner.com/