[Bug testsuite/53028] New: add dg-pedantic

[Bug testsuite/53028] New: add dg-pedantic

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

             Bug #: 53028
           Summary: add dg-pedantic
    Classification: Unclassified
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: testsuite
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: manu@gcc.gnu.org


It would be nice if the testsuite supported a new directive dg-pedantic, to be
used like:

 /* Test for case labels not integer constant expressions but folding
   to integer constants (used in Linux kernel, PR 39613, 52283).  */
 /* { dg-do compile } */
 /* { dg-options "" } */

extern unsigned int u;

void
b (int c)
{
  switch (c)
    {
    case (int) (2  | ((4 < 8) ? 8 : u)): /* { dg-pedantic "case label is not an
integer constant expression" } */
      ;
    }
}

Then, the testsuite will run this testcase two times, one with dg-options and
another with dg-options + -pedantic-errors. In the first case, it should
convert dg-pedantic to dg-bogus,  in the second case it should convert
dg-pedantic to dg-error. 

This feature will avoid a lot of duplicated testcases, and help to test
"-pedantic" much more thoroughly. Unfortunately, my DejaGNU skills are not
sharp enough to figure out this.

[Bug testsuite/53028] add dg-pedantic

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org,
                   |                            |mikestump at comcast dot
                   |                            |net, ro at CeBiTec dot
                   |                            |Uni-Bielefeld.DE
           Severity|normal                      |enhancement

--- Comment #1 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-18 09:20:34 UTC ---
CC testsuite maintainers.

[Bug testsuite/53028] add dg-pedantic

[mikestump at comcast dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #2 from Mike Stump <mikestump at comcast dot net> 2012-04-18 17:35:23 UTC ---
I don't see much value in this.  The primary idea of the gcc testsuite is as a
regression suite.  For a regression, there is just one bit of code that you're
testing, with one set of options.

[Bug testsuite/53028] add dg-pedantic

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #3 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-18 18:25:46 UTC ---
(In reply to comment #2)
> I don't see much value in this.  The primary idea of the gcc testsuite is as a
> regression suite.  For a regression, there is just one bit of code that you're
> testing, with one set of options.

I don't understand this. Maybe I didn't explain myself properly. There are
already hundreds of testcases that are triplicated with the only difference of
not using -pedantic and testing that no warning is given, and using -pedantic
and testing for a warning and using -pedantic-errors and testing that an error
is given. Such testcases imply:

* a duplication of code, which is perhaps not very important nowadays.

* a duplication of creation effort, the developer has to create three testcases
and add the appropriate markers in all of them.

* a duplication of maintainer effort, if a testcase needs to be modified, one
has to check whether a warning/error was caused by -pedantic or not and update
the corresponding duplicated testcases.

Using dg-pedantic will solve the above issues.

This is not very different from the torture testcases that test the same code
with different optimization options or the c-c++-common testcases, but in this
case the meaning of dg-pedantic should change accordingly. Is it possible to
implement this in DejaGNU?

[Bug testsuite/53028] add dg-pedantic

[mikestump at comcast dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #4 from Mike Stump <mikestump at comcast dot net> 2012-04-18 20:01:23 UTC ---
You explained yourself properly.  Just because there are hundreds that do this,
doesn't mean that I necessarily agree with them.  Personally, I'd rip out all
but one of them that either test for the warning or error, it is a cost benefit
valuation.

As for what is possible in DjeaGnu, it is turing complete, so, one can put in
anything for which a turing machine can compute the answer, however, the bar is
higher than that.  The question is, is it a good idea.  To answer this, my
experience tells me, no, not really.  Now, other people's experiences differ,
and I could be swayed by their experience.  For example, please provide 10 PRs
in which we had regressions that could have been caught, but weren't, because
we didn't have such testing.  If none, then, what exactly is the benefit you
see?  For the torture options, history is littered with oodles of such bugs, in
fact, the shear numbers caused people to do the entire torture framework.  It
was put in, to try and permanently stem the tide of such bugs, so they never
happen again, or, at least to reduce in a major way, the shear numbers and to
enhance the reliability of the compiler, and hence, it's reputation. 
Experience tells me, 20-20 hind sight, that the motivation was good and the
results are worth it and that it was the right decision.  It also tells me that
removing the torture options would be a mistake and we would sink back into
where we were before if we wanted to try and remove them.

Now, a counter point.  -pedantic-errors often will not work well because of
things in system header files.  It might be reasonable to #include all the
system headers in a torture setup to ensure that C, C++, pedantic, pedantic
errors, c89, c99 and so on, all work.  The benefit, as failures are found, we
can fixinclude or engineer some sort of solution, so that the user has a nice
experience with the compiler.  I think spending time on this, it a better
cost/benefit choice.

So, to recap, ripping out all but one solve the duplication problem you point
out, it solves the duplication of creation effort you point out, it solves the
duplication of maintaining the testsuite you point out.  It also has the added
benefit of not wasting valuable testing time testing things that never fail.

The above I think is generally true, but I concede there are specific instances
where it is not.

I'll help you understand what I wrote, but I don't know what part you don't
understand.  That dejagnu is a regression suite?  What the word regression
means?  That one test is sufficient to test a pedantic message?

[Bug testsuite/53028] add dg-pedantic

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #5 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-18 21:47:10 UTC ---
(In reply to comment #4)
> 
> So, to recap, ripping out all but one solve the duplication problem you point
> out, it solves the duplication of creation effort you point out, it solves the
> duplication of maintaining the testsuite you point out.  It also has the added
> benefit of not wasting valuable testing time testing things that never fail.
> 

OK, now I understand your point of view. I agree to a certain extent, in the
sense that the current cost/benefit effort is not worth it. If we had already
something like dg-pedantic, the human overhead disappears, but then one could
argue that it is still not worth the extra testing time. Unfortunately, such
testcases are still required during the review process. Well, since you are
testsuite maintainer, I can try to point to this report in the future as to why
I didn't bother to add them. ;-)

[Bug testsuite/53028] add dg-pedantic

[mikestump at comcast dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #6 from Mike Stump <mikestump at comcast dot net> 2012-04-18 22:42:55 UTC ---
So, do you have a pointer to where a maintainer said that they require 3
duplicates for a piece of work?  For all similar future work?  They usually
say, please include a testcase, meaning, one testcase.  One is certainly
reasonable.  For some work, for some cases, they might want 2 or 3 testcases,
but, those I suspect should be rare to very rare.  The requirements for 3 for
that change should not be read as to be generally require 3 for all future
changes...  or, at least, that'd be my interpretation.

[Bug testsuite/53028] add dg-pedantic

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #7 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-19 07:03:30 UTC ---
grep -F "pedantic-errors" testsuite/gcc.dg/*.c

Most of those testcases are duplicated or triplicated.

Another alternative could be if -pedantic warnings always were associated to an
option, then dg-warning tried to match that option. That would avoid having to
add three testcases.

[Bug testsuite/53028] add dg-pedantic

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #8 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-04-23 21:38:00 UTC ---
On Wed, 18 Apr 2012, mikestump at comcast dot net wrote:

> I don't see much value in this.  The primary idea of the gcc testsuite is as a
> regression suite.  For a regression, there is just one bit of code that you're
> testing, with one set of options.

For more than a decade it's been established that the testsuite should 
verify that features or fixes added work as intended; it's not just 
testing a particular bug fix, but comprehensively covering the area of a 
change.  If the intended semantics are pedwarn-if-pedantic, that does mean 
testing what happens in three cases.  If it's a matter of differences 
between standard versions, similarly, testing for each version (and see 
the discussion referenced in 
<http://gcc.gnu.org/ml/gcc-patches/2000-07/msg00262.html> when I started 
adding such tests).

I think features such as dg-pedwarn-if-pedantic or similar would be a 
useful addition to the testsuite infrastructure.

[Bug testsuite/53028] add dg-pedantic

[mikestump at comcast dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #9 from Mike Stump <mikestump at comcast dot net> 2012-04-24 00:31:35 UTC ---
Since little proof was added to support the assertion that the additional
testing is useful, I can remain skeptical about it, though, the CFE people
certainly are free to require it, what they say goes.  I like the idea of
testing new code, just I think it could be over done.

I'll give you an example, I think we all can agree on.

Let's say we add a warning.  The code goes in, and a single testcase:

main() {
    i = code to produce warning;  // warning
}

but, we don't also test:

-pedantic-errors

main() {
    i = code to produce warning; // warning
}

even though, it is reasonable.  We could, but don't.  Why don't we, because it
isn't worth the testing time and the maintenance time.  Testing it once, and
assuming that no one will accidentally change the warning is reasonable.  We
also don't test that the warning goes away with -w.  We don't test the warning
turns into an error with -Werror.

How many times has one of these tests caught someone modifying a pedantic into
a warning?  How many times has one of these tests caught someone modifying a
pedantic into an error?  How many times did someone modify a pedantic in one of
the two ways because a testcase wasn't present?

Feel free to use your best recollection for the above answers.  I can't help
but think the numbers are so low, as to not be worth the cost of the additional
testcases.

[Bug testsuite/53028] add dg-pedantic

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #10 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-28 00:02:13 UTC ---
(In reply to comment #9)
> also don't test that the warning goes away with -w.  We don't test the warning
> turns into an error with -Werror.

Don't we?

http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01778.html

> How many times has one of these tests caught someone modifying a pedantic into
> a warning?  How many times has one of these tests caught someone modifying a
> pedantic into an error?  How many times did someone modify a pedantic in one of
> the two ways because a testcase wasn't present?

The current practice is a burden for developers and for running the testsuite.
Adding dg-pedantic will make it only a burden for running the testsuite (but it
will actually make the testsuite smaller!). Perhaps we can devise an indirect
way to test that a warning is enabled by a certain option. We could have 
{ dg-warning-with-option "Wlong-long" "ISO C90 does not support 'long long'" },
which will match the option shown with -fdiagnostics-show-option. Can we at
least have this?

> Feel free to use your best recollection for the above answers.  I can't help
> but think the numbers are so low, as to not be worth the cost of the additional
> testcases.

Fair enough. But the duplicated testcases are still being added!

[Bug testsuite/53028] add dg-pedantic

[mikestump at comcast dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53028

--- Comment #11 from Mike Stump <mikestump at comcast dot net> 2012-04-30 01:08:24 UTC ---
>> also don't test that the warning goes away with -w.  We don't test the warning
>> turns into an error with -Werror.
> 
> Don't we?
> 
> http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01778.html

Two comments.  First, this, at most can test just one case, I mean, we don't
test all cases.  Second, I don't see -Werror anywhere.  I don't see -w
anywhere.  Maybe I missed how these test these two options...

> Perhaps we can devise an indirect
> way to test that a warning is enabled by a certain option. We could have 
> { dg-warning-with-option "Wlong-long" "ISO C90 does not support 'long long'" },
> which will match the option shown with -fdiagnostics-show-option. Can we at
> least have this?

I like the idea that we have a way to write one test case instead of three... 
I certainly won't object.

> Fair enough. But the duplicated testcases are still being added!

I'd rather error on the side of letting the FE people do what they want.