[m-rev.] for review: structured MLDS var names
jfischer at opturion.com
Sun Apr 23 22:31:02 AEST 2017
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.
> 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
> 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.
The diff looks fine otherwise.
More information about the reviews