[m-rev.] for treview: global, local and field vars in the MLDS

[Julien Fischer]

Hi Zoltan,

On Thu, 20 Jul 2017, Zoltan Somogyi wrote:

> (The first attempt to send this message bounced as too big, which is why
> two of the attached files are gzipped.)
>
> This diff replaces the old mlds_data_defn type with three separate types
> for the definitions of for global, local and field vars respectively.
> This should allow us to encode significantly tighter invariants in the type system,
> e.g. code handling classes shouldn't have to prepare to handle global variables
> in their fields. However, to keep this diff of a manageable size, this diff
> mostly only creates such opportunities; taking advantage of them will come later.
>
> I want to start on that "later" part soon, so if no-one says that they will review
> the diff in the next day or so, I will commit the diff, and deal with any review
> comments later.

I see you've already done that; I've looked through the diff and don't
have any specific comments.

> The diff passes bootcheck in hlc.gc. For the java and csharp grades, the output
> of the updated compiler is NOT bit-identical with the output of the previous compiler,
> as shown by the output of the make_java_csharp_diff tool I committed earlier,
> which is attached as JAVA_CSHARP_DIFF.

I'll test the C# and Java backends this evening.

> There are four kinds of differences.
>
> 1. The compiler generates Java/C# code for functions in a different order.
> To eliminate such differences, I modified both the base and test compilers
> to sort function definitions before writing them out in mlds_to_{cs,java}.m.
> Therefore this difference is NOT in JAVA_CSHARP_DIFF. The others are.
>
> 2. The new compiler always module-qualifies references to runtime system
> enum constants that are notionally defined in private_builtin.m.

As has been noted elsewhere, those enum constants don't really belong
there.  (I intend to shift them into the runtime at some point.)

> The old compiler abused the MLDS (treating those constants as if they
> were global variables) in a way that module qualified them everywhere
> *except* in private_builtin.java.
>
> 3. Almost *all* of the diff part of JAVA_CSHARP_DIFF consists of the
> following single difference, repeated in lots and lots of times:
>
> - /* generic_type */ java.lang.Object closure = null;
> + /* generic_type */ java.lang.Object closure = closure_arg;
> cord.Cord_1 conv0_HeadVar__3_3 = null;
> - closure = closure_arg;

I suspect that optimization was not occurring in the past precisely
because of the things that this diff addresses.

> I don't know what exactly causes the new compiler to be able to apply
> the optimization (moving the first assignment to a variable into the
> variable's initializer) when the old compiler can't, but since the new code
> is better, I am not motivated to undo this difference.
>
> 4. In both the Java and C# translations of mmc_analysis.m, there is
> what appears to be a more complex version of difference #3.
>
> As far as I know, none of the above should cause any problems, but I am
> of course open to correction. In fact, I think the only parts of this change
> that need review are these four differences, and the parts of the diff
> that deal with the files singled out in the log file. The parts of the diff
> that deal with files that only "conform to the changes above" are boring
> and can be skipped, so the review job is not as big as it appears from
> the size of the diff.

Looks fine otherwise.

Julien.