On Sun, 13 Jan 2008, James Keenan via RT wrote:

> I was studying Jarkko's patch this morning in preparation for applying
> it.  I had a number of concerns.
> 
> 1.  It turned out that at the point the patch was originally submitted,
> HEAD had moved a bit beyond the version against which Jarkko diffed.  I
> had refactored some of the patched code into internal subroutines.  So I
> had to adjust the patch to reflect that.

That's fine.

> 2.  If I understand the thrust of the patch correctly, it says, "Only
> set these warnings if we're using GCC as our C compiler.  Otherwise,
> skip it." 

Yes.  That's the basic idea.  (Later amended to possibly try them if 
you're using icc also.)

>  If that's correct, then the value which we need to consult
> inside the Parrot::Configure object at the start of runstep() is not
> options->{gccversion} but data->{gccversion}.

Fine.  You probably understand the innards of the Parrot::Configure object 
better than anyone else.

>  If I understand
> config/auto/gcc.pm correctly, if a user has GCC set up as her default C
> compiler, then data->{gccversion} will be set to the GCC version number
> in that config step -- regardless of whether a value was set for
> 'gccversion' on the command-line.  So I further revised the patch to
> reflect this understanding.

I suppose so.  Normally, gccversion should indeed be set to the value 
corresponding to the compiler the user will actually be using to compile 
parrot.  I hadn't thought at all about what a command-line override for 
gccversion ought to do.  Offhand, I think it's reasonable to simply ignore 
it.

Anyway, the config/auto/warnings.pm part appears to work out just fine.
The t/steps/auto_warnings*.t all still have the unrelated failure due
to using init::defaults and getting the wrong compiler.

-- 
    Andy Dougherty              [EMAIL PROTECTED]

Reply via email to