Re: [Automake-NG] [PATCH 4/4] [ng] cleanup: after enabling of subdir-objects unconditionally

[Stefano Lattarini]

On 06/08/2012 11:06 AM, Akim Demaille wrote:
> 
> Le 8 juin 2012 à 10:29, Stefano Lattarini a écrit :
> 
>> Few minor cleanups made possible by earlier changes, plus other minor
>> cleanups triggered in cascade.  No semantic change is intended.
> 
>> (LANG_IGNORE, LANG_SUBDIR): Remove.
>
> There should be
> 
> * automake.in:
> 
> there.
>
Oops, consider that fixed.

>> automake.in |  100 
>> +++++++++++++++++++++++------------------------------------
>> 1 file changed, 39 insertions(+), 61 deletions(-)
>>
>> diff --git a/automake.in b/automake.in
>> index 14b328a..7d38465 100644
>> --- a/automake.in
>> +++ b/automake.in
>> @@ -809,9 +802,9 @@ register_language ('name' => 'vala',
>>                 'compile' => '$(VALAC) $(AM_VALAFLAGS) $(VALAFLAGS)',
>>                 'ccer' => 'VALAC',
>>                 'compiler' => 'VALACOMPILE',
>> -               'extensions' => ['.vala'],
>> -               'output_extensions' => sub { (my $ext = $_[0]) =~ s/vala$/c/;
>> -                                            return ($ext,) },
>> +               'extensions' => ['.vala', '.vapi'],
>> +                   # Vala compilation must be handled in a special way.
>> +               'output_extensions' => sub { return () },
> 
> Especially for small functions, I really don't see any point
> in keeping "return".  Just { () } is fine.
>
Or even 'sub {}', now that I think about it.  I used the longer expression
above only for consistency with what is done for the language 'header':

    # Headers.
    register_language ('name' => 'header',
                       'Name' => 'Header',
                       'extensions' => ['.h', '.H', '.hxx', '.h++', '.hh',
                                        '.hpp', '.inc'],
                       # No output.
                       'output_extensions' => sub { return () },
                       # Nothing to do.
                      '_finish' => sub { });

I assume I should not care about such "hobgoblin consistency", so I've made
the change "sub { return () }" => "sub { }" for the 'vala' language.

> 
>> @@ -5068,11 +5057,6 @@ sub check_gnits_standards
>> #
>> # Functions to handle files of each language.
>>
>> -# Each 'lang_X_rewrite($DIRECTORY, $BASE, $EXT)' function follows a
>> -# simple formula: Return value is LANG_SUBDIR if the resulting object
>> -# file should be in a subdir if the source file is, LANG_PROCESS if
>> -# file is to be dealt with, LANG_IGNORE otherwise.
>> -
>> # Much of the actual processing is handled in
>> # handle_single_transform.  These functions exist so that
>> # auxiliary information can be recorded for a later cleanup pass.
>> @@ -5105,33 +5089,34 @@ sub lang_c_rewrite
>>                     uniq_part => 'AM_PROG_CC_C_O per-target');
>>        }
>>     }
>> -
>> -  return LANG_SUBDIR;
>> +  return;
> 
> Pretty useless.
>
Nope: in perl, if you don't use an explicit 'return' at the end of a
subroutine, the value of the last expression (whatever that might be)
is returned:

    $ perl -e 'sub foo { my $x = "oops"; }; print &foo . "\n";'
    oops

So the above return is actually needed -- automake dies of an internal
error if it's not present!  Do you want me to add a comment to clarify
this?

>> }
>>
>> -# Rewrite a single header file.
>> -sub lang_header_rewrite
>> +# Header files are simply ignored.
>> +sub lang_header_ignore { return 1; }
> 
> { 1; }
>
As the Zen goes, "Explicit is better than implicit" :-)
But I'll do the change if you insist.

>> +
>> +# Vala '.vapi' are a kind of header files as well, and should
>> +# not be processed into compilation rules.
>> + sub lang_vala_ignore
>> {
>> -    # Header files are simply ignored.
>> -    return LANG_IGNORE;
>> +    my ($directory, $base, $ext) = @_;
>> +    return ($ext =~ m/\.vapi$/ ? 1 : 0);
> 
> No return, no parens.  etc.
>
See above.  The fact that perl allows us to be obscure is not a good
reason to be obscure.  Removing the return and the three-way conditional
would seriously hurt readability IMO.

Thanks,
  Stefano