[m-rev.] for review: `uppercase' attribute for pragma foreign_export_enum

[Julien Fischer]

On Wed, 27 Feb 2008, Zoltan Somogyi wrote:

> On 27-Feb-2008, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
>>          % Otherwise try to derive a name automatically from the
>>          % constructor name.
>> -        ForeignName1 = UnqualCtorName
>> +        (
>> +            MakeUpperCase = yes,
>> +            ForeignName1 = string.to_upper(UnqualCtorName)
>> +        ;
>> +            MakeUpperCase = no,
>> +            ForeignName1 = UnqualCtorName
>> +        )
>>      ),
>>      ForeignName = Prefix ++ ForeignName1,
>>      (
>
> Why ForeignName1 instead of ForeignNameTail?

Mainly, because that's what was already there.
I've changed it to ForeignNameTail.

>>  :- type export_enum_attributes
>>      --->    export_enum_attributes(
>> -                ee_attr_prefix :: maybe(string)
>> +                ee_attr_prefix :: maybe(string),
>> +                ee_attr_upper  :: bool
>
> I think this field should be of a purpose-specific type, not bool,
> since that would make things like ...

Done.

...

>> +process_export_enum_attribute(ee_attr_prefix(MaybePrefix), !Attributes) :-
>> +    % We have already checked that the prefix attribute is not specified
>> +    % multiple times so it is safe to ignore it in the input here.
>> +    !.Attributes = export_enum_attributes(_, MakeUpperCase),
>> +    !:Attributes = export_enum_attributes(MaybePrefix, MakeUpperCase).
>
> The comment is good, but if it said *what code* was supposed to have done
> this check, that would be better.

It's not so obvious from the diff - it's the code directly above this.
I've added a point to it.

>> [
>> +    foo - "lowercase_foo",
>> +    bar  - "mixed1234_bAr"
>> +]).
>
> Align these.

Done.

Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------