Обсуждение: XML changes broke assert-enabled vcbuild

The latest set of XML changes (I think latest, at least fairly recent)
broke the win32vc build with asserts enabled. The line:
    Assert(fully_escaped || !escape_period);

>From what I can tell, this is because the Assert() puts code (the do {}
loop) *before* the declaration of StringInfoData buf, which is not
permitted.

Attached patch seems to fix this. Can someone confirm this is correct
before I put it in?

//Magnus

On Tue, Feb 13, 2007 at 04:29:16PM +0100, Magnus Hagander wrote:
> The latest set of XML changes (I think latest, at least fairly recent)
> broke the win32vc build with asserts enabled. The line:
>     Assert(fully_escaped || !escape_period);
>
> From what I can tell, this is because the Assert() puts code (the do {}
> loop) *before* the declaration of StringInfoData buf, which is not
> permitted.
>
> Attached patch seems to fix this. Can someone confirm this is correct
> before I put it in?

I just realised I should of course move the comment as well :-) Thus,
the attached patch is more correct.

//Magnus

Magnus Hagander <magnus@hagander.net> writes:
> From what I can tell, this is because the Assert() puts code (the do {}
> loop) *before* the declaration of StringInfoData buf, which is not
> permitted.

This will fail on every ANSI-C compiler, not just vc.  Please fix.
        regards, tom lane

On Tue, Feb 13, 2007 at 10:50:30AM -0500, Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > From what I can tell, this is because the Assert() puts code (the do {}
> > loop) *before* the declaration of StringInfoData buf, which is not
> > permitted.
> 
> This will fail on every ANSI-C compiler, not just vc.  Please fix.

Applied.

//Magnus

Magnus Hagander wrote:
> The latest set of XML changes (I think latest, at least fairly recent)
> broke the win32vc build with asserts enabled. The line:
>     Assert(fully_escaped || !escape_period);
> 
> From what I can tell, this is because the Assert() puts code (the do {}
> loop) *before* the declaration of StringInfoData buf, which is not
> permitted.

Certainly.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > From what I can tell, this is because the Assert() puts code (the
> > do {} loop) *before* the declaration of StringInfoData buf, which
> > is not permitted.
>
> This will fail on every ANSI-C compiler, not just vc.  Please fix.

We seem to have very poor coverage of such compilers in the build farm, 
it seems.  Is the vcbuild ready to support a regular build farm run 
yet?

It turns out that gcc warns about it anyway.  Does anyone have some sort 
of clever recipe to catch warnings more easily than by carefully 
reading the make output or manually grepping build log files or 
something?

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

On Tue, Feb 13, 2007 at 05:23:56PM +0100, Peter Eisentraut wrote:
> Tom Lane wrote:
> > Magnus Hagander <magnus@hagander.net> writes:
> > > From what I can tell, this is because the Assert() puts code (the
> > > do {} loop) *before* the declaration of StringInfoData buf, which
> > > is not permitted.
> >
> > This will fail on every ANSI-C compiler, not just vc.  Please fix.
> 
> We seem to have very poor coverage of such compilers in the build farm, 
> it seems.  Is the vcbuild ready to support a regular build farm run 
> yet?

Backend-wise, yes. It does require some changes to the buildfarm code itself
(can't call make and such), which I beleive Andrew is working on (when
he has free time).

(the vcregress script I just committed was the final "major step"
towards it, I think)

> It turns out that gcc warns about it anyway.  Does anyone have some sort 
> of clever recipe to catch warnings more easily than by carefully 
> reading the make output or manually grepping build log files or 
> something?

Perhaps something we could have the buildfarm do as well, if it can be
automated?

//Magnus

Peter Eisentraut wrote:
> Tom Lane wrote:
> > Magnus Hagander <magnus@hagander.net> writes:
> > > From what I can tell, this is because the Assert() puts code (the
> > > do {} loop) *before* the declaration of StringInfoData buf, which
> > > is not permitted.
> >
> > This will fail on every ANSI-C compiler, not just vc.  Please fix.
> 
> We seem to have very poor coverage of such compilers in the build farm, 
> it seems.  Is the vcbuild ready to support a regular build farm run 
> yet?
> 
> It turns out that gcc warns about it anyway.  Does anyone have some sort 
> of clever recipe to catch warnings more easily than by carefully 
> reading the make output or manually grepping build log files or 
> something?

Yes, I run /src/tools/pgtest, which shows the warning lines at the end,
after the regression tests are run.

--  Bruce Momjian  <bruce@momjian.us>          http://momjian.us EnterpriseDB
http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Magnus Hagander <magnus@hagander.net> writes:
> On Tue, Feb 13, 2007 at 05:23:56PM +0100, Peter Eisentraut wrote:
>> It turns out that gcc warns about it anyway.  Does anyone have some sort 
>> of clever recipe to catch warnings more easily than by carefully 
>> reading the make output or manually grepping build log files or 
>> something?

> Perhaps something we could have the buildfarm do as well, if it can be
> automated?

I tend to do "make >make.out 2>make.err" and then look at make.err.
The normal situation with a gcc build is that make.err contains one or
two warnings due to flex's bad habits.  We could possibly get that down
to zero if we wanted to work at it.  However, most non-gcc compilers
I've looked at generate dozens of mostly-silly warnings, so I'm not sure
if the buildfarm could use this technique or not.
        regards, tom lane