Thread: 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
Attachment
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
Attachment
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