Thread: [Module-build-general] "0" options in compilation lines?
Status: Beta
Brought to you by:
kwilliams
|
From: Ken W. <ke...@ma...> - 2003-02-24 18:04:58
|
Hi Randy,
I noticed this line in the format_compiler_cmd() code from your patch:
return [ grep {defined && $_} (
$spec{cc}, '-c' ,
@{$spec{includes}} ,
@{$spec{cflags}} ,
@{$spec{optimize}} ,
@{$spec{defines}} ,
@{$spec{perlinc}} ,
"-Fo$spec{output}" ,
$spec{source} ,
) ];
I was wondering about the check for boolean truth, which will turn
argument lists like ('-foo', 0) into ('-foo'). Maybe it would be safer
to just use grep {defined()}, or grep {defined() && length()}, or
something? Or are there some zeroes you're specifically trying to
filter out?
-Ken
|
|
From: Randy W. S. <Ra...@Th...> - 2003-02-26 00:56:41
|
On 2/24/2003 1:04 PM, Ken Williams wrote:
> Hi Randy,
>
> I noticed this line in the format_compiler_cmd() code from your patch:
>
> return [ grep {defined && $_} (
> $spec{cc}, '-c' ,
> @{$spec{includes}} ,
> @{$spec{cflags}} ,
> @{$spec{optimize}} ,
> @{$spec{defines}} ,
> @{$spec{perlinc}} ,
> "-Fo$spec{output}" ,
> $spec{source} ,
> ) ];
>
> I was wondering about the check for boolean truth, which will turn
> argument lists like ('-foo', 0) into ('-foo'). Maybe it would be safer
> to just use grep {defined()}, or grep {defined() && length()}, or
> something? Or are there some zeroes you're specifically trying to
> filter out?
>
> -Ken
Your right. Bad habbit. I know it's bad for me, but I can't stop...
I can't think of anyway in which it might fail, but it should be changed.
Randy.
|
|
From: Ken W. <ke...@ma...> - 2003-02-26 03:24:21
|
On Tuesday, February 25, 2003, at 06:56 PM, Randy W. Sims wrote:
> On 2/24/2003 1:04 PM, Ken Williams wrote:
>> Hi Randy,
>> I noticed this line in the format_compiler_cmd() code from your patch:
>> return [ grep {defined && $_} (
>> $spec{cc}, '-c' ,
>> @{$spec{includes}} ,
>> @{$spec{cflags}} ,
>> @{$spec{optimize}} ,
>> @{$spec{defines}} ,
>> @{$spec{perlinc}} ,
>> "-Fo$spec{output}" ,
>> $spec{source} ,
>> ) ];
>> I was wondering about the check for boolean truth, which will turn
>> argument lists like ('-foo', 0) into ('-foo'). Maybe it would be
>> safer to just use grep {defined()}, or grep {defined() && length()},
>> or something? Or are there some zeroes you're specifically trying to
>> filter out?
>> -Ken
>
> Your right. Bad habbit. I know it's bad for me, but I can't stop...
>
> I can't think of anyway in which it might fail, but it should be
> changed.
How about I just change it to
return [
$spec{cc}, '-c' ,
@{$spec{includes}} ,
@{$spec{cflags}} ,
@{$spec{optimize}} ,
@{$spec{defines}} ,
@{$spec{perlinc}} ,
"-Fo$spec{output}" ,
$spec{source} ,
];
? Seems like we'll want to know if one of those values is undefined
anyway, so we'll want to trigger the warning if warnings are turned on.
-Ken
|
|
From: Randy W. S. <Ra...@Th...> - 2003-02-27 01:52:36
Attachments:
patch
|
On 2/25/2003 10:24 PM, Ken Williams wrote:
>
> On Tuesday, February 25, 2003, at 06:56 PM, Randy W. Sims wrote:
>
>> On 2/24/2003 1:04 PM, Ken Williams wrote:
>>
>>> I was wondering about the check for boolean truth, which will turn
>>> argument lists like ('-foo', 0) into ('-foo'). Maybe it would be
>>> safer to just use grep {defined()}, or grep {defined() && length()},
>>> or something? Or are there some zeroes you're specifically trying to
>>> filter out?
>>> -Ken
>>
>>
>> Your right. Bad habbit. I know it's bad for me, but I can't stop...
>>
>> I can't think of anyway in which it might fail, but it should be changed.
>
>
> How about I just change it to
>
> return [
> $spec{cc}, '-c' ,
> @{$spec{includes}} ,
> @{$spec{cflags}} ,
> @{$spec{optimize}} ,
> @{$spec{defines}} ,
> @{$spec{perlinc}} ,
> "-Fo$spec{output}" ,
> $spec{source} ,
> ];
>
> ? Seems like we'll want to know if one of those values is undefined
> anyway, so we'll want to trigger the warning if warnings are turned on.
>
> -Ken
That wouldn't work as it's written. Just before the return there is a
call to write_*_script() which delete()s the elements in the hash that
it is able to write to the script file, so it needs to be grep{defined
&& length} to avoid warnings about the no longer existing elements.
I've attached a patch that fixes that and fixes a couple of
inconsistencies in the way I was access $self->{config} (i.e. in most
places I was accessing through the convenience variable $cf while in a
couple of spots I spelled it out $self->{config}).
In regards to warning users about undefined entries, I think it is valid
for some options to be undefined, but I really don't know enough about
typical usage across the various platforms to test the validity of
options. I need to study modules like MakeMaker and others that use the
compiler to see what they do, as well as studing some Makefile.PLs and
the generated Makefiles to try to get a better grasp on how the compiler
and linker are generally used.
Randy.
|
|
From: Ken W. <ke...@ma...> - 2003-02-27 02:59:53
|
On Wednesday, February 26, 2003, at 07:52 PM, Randy W. Sims wrote:
>
> That wouldn't work as it's written. Just before the return there is a
> call to write_*_script() which delete()s the elements in the hash that
> it is able to write to the script file, so it needs to be grep{defined
> && length} to avoid warnings about the no longer existing elements.
Well, if you delete($spec{foo}), then @{$spec{foo}} returns an empty
list, not something with undef elements in it.
> I've attached a patch that fixes that and fixes a couple of
> inconsistencies in the way I was access $self->{config} (i.e. in most
> places I was accessing through the convenience variable $cf while in a
> couple of spots I spelled it out $self->{config}).
Good, $cf is nicer there. No possibility of misspelling 'congif'
either. =)
> In regards to warning users about undefined entries, I think it is
> valid for some options to be undefined, but I really don't know enough
> about typical usage across the various platforms to test the validity
> of options. I need to study modules like MakeMaker and others that use
> the compiler to see what they do, as well as studing some Makefile.PLs
> and the generated Makefiles to try to get a better grasp on how the
> compiler and linker are generally used.
My sense is that the cmd-line flags would be better left out than given
undef values. It doesn't seem to make much sense to do system('gcc',
'-foo', undef).
I'll apply your patch as-is, if for no other reason than the fact that
you can test things out, and I can't, so I ought to trust you more on
the code than myself. =)
-Ken
|