[RFC] enabling -fshow-column by default

[RFC] enabling -fshow-column by default

[Aldy Hernandez]

Hi folks.

Before I merge the diagnostics branch I'd like to enable it on the
testsuite to get us all in the habit of at least being aware of columns.

Joseph Myers suggested enabling it in the compiler instead of the
testsuite.  Are there any big objections to this?

Aldy

Re: [RFC] enabling -fshow-column by default

[Manuel López-Ibáñez]

2009/5/20 Aldy Hernandez <aldyh@redhat.com>:
> Hi folks.
>
> Before I merge the diagnostics branch I'd like to enable it on the
> testsuite to get us all in the habit of at least being aware of columns.
>
> Joseph Myers suggested enabling it in the compiler instead of the
> testsuite.  Are there any big objections to this?

My only worry is that the testsuite may confuse column and line
numbers and pass/fail tests because of it.

Index: gcc/testsuite/gcc.dg/bogus-column.c
===================================================================
--- gcc/testsuite/gcc.dg/bogus-column.c	(revision 0)
+++ gcc/testsuite/gcc.dg/bogus-column.c	(revision 0)
@@ -0,0 +1,11 @@
+/* This testcase checks that line numbers are not confused with column
+   numbers.  */
+/* { dg-do compile } */
+/* { dg-options "-fshow-column -fms-extensions -pedantic" } */
+
+/* { dg-bogus "ISO C doesn't support unnamed" "bogus" { target *-*-* } 22 } */
+
+struct {
+  struct a { int x; }; /* { dg-warning "22:ISO C doesn't support unnamed" } */
+  int bar;
+} hot;


Cheers,

Manuel.

Re: [RFC] enabling -fshow-column by default

[Aldy Hernandez]

On Wed, May 20, 2009 at 04:39:00PM +0200, Manuel L?pez-Ib??ez wrote:
> 2009/5/20 Aldy Hernandez <aldyh@redhat.com>:
> > Hi folks.
> >
> > Before I merge the diagnostics branch I'd like to enable it on the
> > testsuite to get us all in the habit of at least being aware of columns.
> >
> > Joseph Myers suggested enabling it in the compiler instead of the
> > testsuite. ?Are there any big objections to this?
> 
> My only worry is that the testsuite may confuse column and line
> numbers and pass/fail tests because of it.

Janis has a patch for the testsuite to handle all this.

I am testing her patch with some of my own testsuite changes as well,
and we're basically down to no regressions.

Re: [RFC] enabling -fshow-column by default

[Jonathan Wakely]

2009/5/20 Aldy Hernandez:
>>
>> My only worry is that the testsuite may confuse column and line
>> numbers and pass/fail tests because of it.
>
> Janis has a patch for the testsuite to handle all this.

I'm seeing exactly this in the libstdc++ testsuite with some new tests
I've written - is a fix on the way soon?

I have something like:

/*33*/  foo<int> p1;            // { dg-error "no match" }
/*34*/  foo<int&> p2;           // { dg-error "no match" }

and the error for p2 has line:column as 34:33: which matches the first
dg-error, so I get a FAIL on line 34

Jonathan

Re: [RFC] enabling -fshow-column by default

[Aldy Hernandez]

On Fri, Jun 05, 2009 at 12:09:57AM +0100, Jonathan Wakely wrote:
> 2009/5/20 Aldy Hernandez:
> >>
> >> My only worry is that the testsuite may confuse column and line
> >> numbers and pass/fail tests because of it.
> >
> > Janis has a patch for the testsuite to handle all this.
> 
> I'm seeing exactly this in the libstdc++ testsuite with some new tests
> I've written - is a fix on the way soon?

The fix is already in the tree.

Which test is this?  Can you send it to me?

Aldy

Re: [RFC] enabling -fshow-column by default

[Janis Johnson]

On Fri, 2009-06-05 at 10:37 -0400, Aldy Hernandez wrote:
> On Fri, Jun 05, 2009 at 12:09:57AM +0100, Jonathan Wakely wrote:
> > 2009/5/20 Aldy Hernandez:
> > >>
> > >> My only worry is that the testsuite may confuse column and line
> > >> numbers and pass/fail tests because of it.
> > >
> > > Janis has a patch for the testsuite to handle all this.
> > 
> > I'm seeing exactly this in the libstdc++ testsuite with some new tests
> > I've written - is a fix on the way soon?
> 
> The fix is already in the tree.
> 
> Which test is this?  Can you send it to me?

I think the libstdc++ testsuite doesn't use the overrides for dg-error
and friends and so isn't handling the column numbers in the new way.
I'll take a look, but it might be awhile before I get to it.

Janis

Re: [RFC] enabling -fshow-column by default

[Jonathan Wakely]

2009/6/5 Janis Johnson:
>
> I think the libstdc++ testsuite doesn't use the overrides for dg-error
> and friends and so isn't handling the column numbers in the new way.
> I'll take a look, but it might be awhile before I get to it.

Thanks, Janis. I have added -fno-show-column to
libstdc++-v3/scripts/testsuite_flags for now.

Jonathan

Re: [RFC] enabling -fshow-column by default

[Jonathan Wakely]

2009/6/5 Aldy Hernandez:
>
> Which test is this?  Can you send it to me?

It tests a header that isn't checked in yet, so sending the test alone
wouldn't help much :)

I'll try to come up with a self-contained example tomorrow.

Jonathan

Re: [RFC] enabling -fshow-column by default

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

2009/6/5 Jonathan Wakely:
> 2009/6/5 Aldy Hernandez:
>>
>> Which test is this?  Can you send it to me?
>
> It tests a header that isn't checked in yet, so sending the test alone
> wouldn't help much :)
>
> I'll try to come up with a self-contained example tomorrow.

The attached test will FAIL with a trunk build from yesterday, with
the output in the second attachment.

The test for errors on line 11 fails because the error is at :11:9:
which matches the test for errors on line 9.

Jonathan

[-- Attachment #2: columns_neg.cc --]
[-- Type: text/plain, Size: 175 bytes --]

// { dg-do compile }

struct X
{
};

int main()
{
  X x1(0);  // { dg-error "no match" }

  X x2(0);  // { dg-error "no match" }

  return 0;
}
// { dg-excess-errors "note" }

[-- Attachment #3: fail.log --]
[-- Type: application/octet-stream, Size: 2254 bytes --]

/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc: In function 'int main()':
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:9:9: error: no matching function for call to 'X::X(int)'
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note: candidates are: X::X()
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note:                 X::X(const X&)
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:11:9: error: no matching function for call to 'X::X(int)'
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note: candidates are: X::X()
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note:                 X::X(const X&)
compiler exited with status 1
output is:
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc: In function 'int main()':
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:9:9: error: no matching function for call to 'X::X(int)'
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note: candidates are: X::X()
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note:                 X::X(const X&)
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:11:9: error: no matching function for call to 'X::X(int)'
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note: candidates are: X::X()
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note:                 X::X(const X&)

PASS: 30_threads/columns_neg.cc  (test for errors, line 9)
FAIL: 30_threads/columns_neg.cc  (test for errors, line 11)
XFAIL: 30_threads/columns_neg.cc (test for excess errors)
Excess errors:
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note: candidates are: X::X()
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note:                 X::X(const X&)
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note: candidates are: X::X()
/home/jwakely/src/gcc/libstdc++-v3/testsuite/30_threads/columns_neg.cc:4:1: note:                 X::X(const X&)

Re: [RFC] enabling -fshow-column by default

[Aldy Hernandez]

On Thu, Jun 11, 2009 at 03:09:48PM +0100, Jonathan Wakely wrote:
> 2009/6/5 Jonathan Wakely:
> > 2009/6/5 Aldy Hernandez:
> >>
> >> Which test is this? ?Can you send it to me?
> >
> > It tests a header that isn't checked in yet, so sending the test alone
> > wouldn't help much :)
> >
> > I'll try to come up with a self-contained example tomorrow.
> 
> The attached test will FAIL with a trunk build from yesterday, with
> the output in the second attachment.
> 
> The test for errors on line 11 fails because the error is at :11:9:
> which matches the test for errors on line 9.
> 
> Jonathan

> // { dg-do compile }
> 
> struct X
> {
> };
> 
> int main()
> {
>   X x1(0);  // { dg-error "no match" }
> 
>   X x2(0);  // { dg-error "no match" }
> 
>   return 0;
> }
> // { dg-excess-errors "note" }


The libstdc++ testsuite doesn't use the overrides for dg-error et al.  I
believe Janis is looking into this.

In the meantime, can you just put the column number in the dg-error
message?  i.e:

	// { dg-error "XX:no match" }

This will match the message correctly, and eventually all error messages
should have matching columns.

Aldy

Re: [RFC] enabling -fshow-column by default

[Jonathan Wakely]

2009/6/11 Aldy Hernandez:
>
> In the meantime, can you just put the column number in the dg-error
> message?  i.e:
>
>        // { dg-error "XX:no match" }
>
> This will match the message correctly, and eventually all error messages
> should have matching columns.

That works great, thanks, Aldy.

Jonathan

Re: [RFC] enabling -fshow-column by default

[Ian Lance Taylor]

Aldy Hernandez <aldyh@redhat.com> writes:

> Before I merge the diagnostics branch I'd like to enable it on the
> testsuite to get us all in the habit of at least being aware of columns.
>
> Joseph Myers suggested enabling it in the compiler instead of the
> testsuite.  Are there any big objections to this?

I agree in principle that we should enable -fshow-column by default.
Obviously, it will break some existing scripts, but I tend to think that
for something like this that is OK.  Any scripts which break are already
slightly broken in that preprocessor error messages already display a
column number.

If you haven't already done so, please check that the emacs next-error
function is not affected by this.

When we have good caret diagnostics it would also be nice to enable them
by default.  However, that is a much bigger change.  Does anybody have
an opinion on that?

Ian

Re: [RFC] enabling -fshow-column by default

[Manuel López-Ibáñez]

2009/5/20 Ian Lance Taylor <iant@google.com>:

> When we have good caret diagnostics it would also be nice to enable them
> by default.  However, that is a much bigger change.  Does anybody have
> an opinion on that?

Why don't have that discussion when (and if) caret diagnostics are
available? I don't think anyone is actively working on it, so it will
just be a waste of bandwidth.

Cheers,

Manuel.

Re: [RFC] enabling -fshow-column by default

[Joseph S. Myers]

On Wed, 20 May 2009, Ian Lance Taylor wrote:

> When we have good caret diagnostics it would also be nice to enable them
> by default.  However, that is a much bigger change.  Does anybody have
> an opinion on that?

Yes, that tripling the size of output in a cascade of diagnostics is a bad 
idea.  Possibly disable them by default after the first few errors (or a 
larger number of warnings) if enabled by default (not explicitly) up to 
then?  Also, work with the GNU Coding Standards maintainers on any 
conventions for caret diagnostics in GNU software.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC] enabling -fshow-column by default

[Tom Tromey]

>>>>> "Ian" == Ian Lance Taylor <iant@google.com> writes:

Ian> If you haven't already done so, please check that the emacs next-error
Ian> function is not affected by this.

This definitely works.

Tom