[m-rev.] for review: structured MLDS var names

Julien Fischer jfischer at opturion.com
Sun Apr 23 22:31:02 AEST 2017


Hi Zoltan,

On Sat, 22 Apr 2017, Zoltan Somogyi wrote:

> For review by anyone, but Paul should definitely look
> at the log message, since the main intention of the diff
> is to make his job easier.

...

> Use a structured representation for MLDS variables.
> 
> compiler/mlds.m:
>     Replace the old definition of mlds_var_name, which was a string
>     with an optional integer. The integer was intended to be the number
>     of a HLDS variable, while auxiliary variables created by the compiler,
>     which do not correspond to a HLDS variable, would not have the optional
>     integer.
>
>     This design has a couple of minor problems. The first is that there is
>     no place in the compiler where all the variable names are visible at once,
>     and without such a place, we cannot be sure that two names constructed
>     for different purposes don't accidentally end up with the same name.
>     The probability of such a clash used to be astronomically small

See bug #92 <https://bugs.mercurylang.org/view.php?id=92>.

>     (which is why this hasn't been a problem), but it was not zero.

>     The second problem is that several kinds of compiler-created MLDS variables
>     want to have numerical suffixes too, usually with the suffix being a
>     unique sequence number used as a means of disambiguation. Most of the
>     places where these were created put the numerical suffix into the name
>     string itself, while some put the sequence number as the optional integer.
>
>     As it happens, neither of those actions is good when one wants to take
>     the independently generated MLDS code of several procedures in an SCC
>     and merge them into a single piece of MLDS code. For this, we want to
>     rename apart both the HLDS variable numbers and the sequence numbers.
>     Having the sequence number baked into the strings themselves obviously
>     makes such renumbering unnecessarily hard, while having sequence numbers
>     in the slots intended for HLDS variable numbers makes the job impossible
>     to do safely.
>
>     This diff switches to a new representation of mlds_var_names that
>     has a separate function symbol for each different "origin story"
>     that is possible for MLDS variables. This addresses both problems.
>
>     The single predicate that converts this structured representation
>     to a string is the place where we can ensure that two semantically
>     different MLDS variables never get translated to the same string.
>     The current version of this predicate does *not* offer this guarantee,
>     but later versions could.
>
>     And having all the integers used in mlds_var_names for different purposes
>     stored as arguments of different function symbols (that clearly indicate
>     their meaning) makes it possible to rename apart different sets
>     of MLDS variables easily and reliably.
>
>     Move the code for converting mlds_var_names from ml_code_util.m to here,
>     to make it easier to maintain it together with the mlds_var_name type.
>

...

> diff --git a/compiler/mlds.m b/compiler/mlds.m
> index ffa7fb1..544f601 100644
> --- a/compiler/mlds.m
> +++ b/compiler/mlds.m

...

> +:- type mlds_compiler_var

....

> +    ;       mcv_mr_value
> +    ;       mcv_data_tag
> +    ;       mcv_enum_const(string)
> +    ;       mcv_sectag_const(string)
> +            % These MLDS variables represent member variables in the classes
> +            % we generate with --high-level-data for discriminated union types
> +            % (both enum and non-enum). I (zs) don't know exactly what
> +            % they are user for.

s/user/used/

The diff looks fine otherwise.

Julien.


More information about the reviews mailing list