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

[Zoltan Somogyi]

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?

>  :- 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 ...

> +default_export_enum_attributes = export_enum_attributes(no, no).

... more understandable.

> +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.

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

Align these.

Otherwise, the diff is fine.

Zoltan.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------