Bug 13683

Summary: [3.3 Regression] bogus warning about passing non-PODs through ellipsis
Product: gcc Reporter: Giovanni Bajo <giovannibajo>
Component: c++Assignee: Giovanni Bajo <giovannibajo>
Status: RESOLVED FIXED    
Severity: minor CC: gcc-bugs
Priority: P2 Keywords: diagnostic
Version: 3.4.0   
Target Milestone: 3.3.3   
Host: Target:
Build: Known to work: 2.95.3
Known to fail: 3.3.3 3.4.0 4.0.0 Last reconfirmed: 2004-01-14 21:40:39

Description Giovanni Bajo 2004-01-14 16:56:47 UTC
The following code emits a bogus warning:

-----------------------------------------------------
struct B {};
struct NonPOD : B {};

struct A
{
  static int check(...);
  static NonPOD GetNonPOD(void);
  enum { value = sizeof(A::check(A::GetNonPOD())) };
};
-----------------------------------------------------
bogus.cc:8: warning: cannot pass objects of non-POD type `struct NonPOD' 
through `...'; call will abort at runtime


The problem is that GCC fails to realize that the function call will never 
happen because it's done in the context of a sizeof() expression. Thus, the 
warning should not be emitted. Notice that this warning is on by default, and I 
don't even know if there is a way to shut it off.

Regression since 2.95 (where the warning probably didn't exist, but from an 
user standpoint it's a regression).

BTW this was noticed with Boost code, so it's real world stuff. Luckily, the 
warning is not affected by -Werror. I rate this as a very minor regression, but 
it would be great if we could fix it someday.
Comment 1 Andrew Pinski 2004-01-14 17:31:38 UTC
I do not know but I will say the warning is partly correct even though it is not called at runtime.
Comment 2 Wolfgang Bangerth 2004-01-14 21:40:39 UTC
Confirmed. I'd say don't do that. I don't think this deserves a particularly 
high importance. 
 
Did you set the milestone because you wanted to fix it yourself? 
 
W. 
Comment 3 Giovanni Bajo 2004-01-15 02:07:29 UTC
Subject: Re:  [3.3/3.4 Regression] bogus warning about passing non-PODs through ellipsis

bangerth at dealii dot org wrote:

> Did you set the milestone because you wanted to fix it yourself?

I'll surely look into it, but it's also a regression even if it's very minor.

Giovanni Bajo


Comment 4 Mark Mitchell 2004-01-16 18:12:48 UTC
I believe that it's reasonable to issue a warning about the code in question,
despite the fact that the code appears as the operand to a sizeof.

Givoanni, if you disagree, please raise the issue with Jason.  If you both agree
that this really is a problem, reopen this PR.
Comment 5 Giovanni Bajo 2004-01-19 14:54:15 UTC
I spoke with Jason, and he agreed that we can at least try to tackle this 
issue. Checking skip_evaluation in in convert_arg_to_ellipsis might be enough.
Comment 6 Mark Mitchell 2004-01-20 07:54:03 UTC
Yes, the question to me is not "can we change the compiler's behavior", but
"should we?"
Comment 7 Giovanni Bajo 2004-01-20 12:05:29 UTC
Subject: Re:  [3.3/3.4 Regression] bogus warning about passing non-PODs through ellipsis

mmitchel at gcc dot gnu dot org wrote:

> Yes, the question to me is not "can we change the compiler's behavior"
> , but "should we?"

Passing a non-POD object through ellipsis is undefined behaviour *if* the call
is done. In our situation, there is absolutely no call being performed (nor
code generated where we abort), so there is no undefined behaviour. We're not
warning of an existing potential issue, because the call will simply never
happen. We could even argue that the diagnostic is misleading - by saying "the
code will abort at runtime", we trick the user into believing that there is
indeed a function call.

Giovanni Bajo


Comment 8 Gabriel Dos Reis 2004-01-20 13:55:49 UTC
Subject: Re:  [3.3/3.4 Regression] bogus warning about passing non-PODs through ellipsis

"giovannibajo at libero dot it" <gcc-bugzilla@gcc.gnu.org> writes:

| mmitchel at gcc dot gnu dot org wrote:
| 
| > Yes, the question to me is not "can we change the compiler's behavior"
| > , but "should we?"
| 
| Passing a non-POD object through ellipsis is undefined behaviour *if* the call
| is done. In our situation, there is absolutely no call being performed (nor
| code generated where we abort), so there is no undefined behaviour.

This is a *warning* not an error, and warnings are just about that.
If we knew it were actually undefined, we would probably be issueing
an error.

I can understand your request in the case of sizeof but I do not
understand it as rephrased as above.  How precisely do you define call
being performed or code generated?  

   if (0) {
       pass through ellipsis
   } else {
      do something else
   }

Remember the "0" can come from macro expansion like "HAVE_FEATURE" and
that people compile such codes so as to get as maximum of compiler
check as they can.

(And if you asked me, I consider SFINAE brittle code ;-)

| We're not
| warning of an existing potential issue, because the call will simply never
| happen. We could even argue that the diagnostic is misleading - by saying "the
| code will abort at runtime", we trick the user into believing that there is
| indeed a function call.

We can nitpick on how the actual wording should be, but the basic
issue is not there. 
Comment 9 Giovanni Bajo 2004-01-20 14:11:52 UTC
Subject: Re:  [3.3/3.4 Regression] bogus warning about passing non-PODs through ellipsis

gdr at integrable-solutions dot net wrote:

>> Passing a non-POD object through ellipsis is undefined behaviour
>> *if* the call is done. In our situation, there is absolutely no call being
>> performed (nor code generated where we abort), so there is no
>> undefined behaviour.

> I can understand your request in the case of sizeof but I do not
> understand it as rephrased as above.  How precisely do you define call
> being performed or code generated?

I was thinking of something along the lines of "potentially evaluated",
[basic.def.odr]/2.  Anyway, if we both agree on my request for the sizeof()
case, I'm happy with it, I'll let you pick my explanation of it which is more
correct, or please do provide your own reasoning for this.

Giovanni Bajo


Comment 10 Gabriel Dos Reis 2004-01-20 15:03:29 UTC
Subject: Re:  [3.3/3.4 Regression] bogus warning about passing non-PODs through ellipsis

"giovannibajo at libero dot it" <gcc-bugzilla@gcc.gnu.org> writes:

| ------- Additional Comments From giovannibajo at libero dot it  2004-01-20 14:11 -------
| Subject: Re:  [3.3/3.4 Regression] bogus warning about passing non-PODs through ellipsis
| 
| gdr at integrable-solutions dot net wrote:
| 
| >> Passing a non-POD object through ellipsis is undefined behaviour
| >> *if* the call is done. In our situation, there is absolutely no call being
| >> performed (nor code generated where we abort), so there is no
| >> undefined behaviour.
| 
| > I can understand your request in the case of sizeof but I do not
| > understand it as rephrased as above.  How precisely do you define call
| > being performed or code generated?
| 
| I was thinking of something along the lines of "potentially evaluated",
| [basic.def.odr]/2.

Oh yeah, go some paragraphs down, and you'll see that thw whole thing
is a little bit confused.  And remember, most people want intelligible
diagnostics and are not well versed in "standardese" (i.e. common
words with slightly different meaning).

|  Anyway, if we both agree on my request for the sizeof()
| case, I'm happy with it, I'll let you pick my explanation of it which is more
| correct, or please do provide your own reasoning for this.

As I said earlier I understand the request for sizeof() as it is an
*integral constant expression* whose value does not involve a runtime
semantics and is well defined.

-- Gaby
Comment 11 Giovanni Bajo 2004-01-30 13:14:30 UTC
Mine!
Comment 12 GCC Commits 2004-01-30 15:08:41 UTC
Subject: Bug 13683

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	giovannibajo@gcc.gnu.org	2004-01-30 15:08:37

Modified files:
	gcc/cp         : ChangeLog call.c 

Log message:
	PR c++/13683
	* call.c (convert_arg_to_ellipsis): Don't emit a warning if within
	a sizeof expression.block

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.3918&r2=1.3919
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&r1=1.455&r2=1.456

Comment 13 GCC Commits 2004-01-30 15:12:49 UTC
Subject: Bug 13683

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	giovannibajo@gcc.gnu.org	2004-01-30 15:12:45

Modified files:
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/template: sizeof6.C 

Log message:
	PR c++/13683
	* g++.dg/template/sizeof6.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3443&r2=1.3444
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/template/sizeof6.C.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 14 Giovanni Bajo 2004-01-30 15:14:45 UTC
Patch here:
http://gcc.gnu.org/ml/gcc-patches/2004-01/msg03395.html

Committed to mainline, testing in progress for 3.4.
Comment 15 Gabriel Dos Reis 2004-01-30 15:59:33 UTC
Subject: Re:  [3.3/3.4 Regression] bogus warning about passing non-PODs through ellipsis

"giovannibajo at libero dot it" <gcc-bugzilla@gcc.gnu.org> writes:

| ------- Additional Comments From giovannibajo at libero dot it  2004-01-30 15:14 -------
| Patch here:
| http://gcc.gnu.org/ml/gcc-patches/2004-01/msg03395.html
| 
| Committed to mainline, testing in progress for 3.4.

OK for 3.3.3

-- Gaby
Comment 16 GCC Commits 2004-01-30 16:21:31 UTC
Subject: Bug 13683

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	giovannibajo@gcc.gnu.org	2004-01-30 16:21:26

Modified files:
	gcc/cp         : ChangeLog call.c 

Log message:
	PR c++/13683
	* call.c (convert_arg_to_ellipsis): Don't emit a warning if within
	a sizeof expression.block

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3892.2.22&r2=1.3892.2.23
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.452.2.3&r2=1.452.2.4

Comment 17 GCC Commits 2004-01-30 16:24:04 UTC
Subject: Bug 13683

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	giovannibajo@gcc.gnu.org	2004-01-30 16:24:00

Modified files:
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/template: sizeof6.C 

Log message:
	PR c++/13683
	* g++.dg/template/sizeof6.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.33&r2=1.3389.2.34
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/template/sizeof6.C.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1

Comment 18 Giovanni Bajo 2004-01-30 16:24:53 UTC
Now it's only a 3.3 regression. I'm testing the patch there and waiting for 
Gaby's approval.
Comment 19 Gabriel Dos Reis 2004-01-30 17:11:44 UTC
Subject: Re:  [3.3 Regression] bogus warning about passing non-PODs through ellipsis

"giovannibajo at libero dot it" <gcc-bugzilla@gcc.gnu.org> writes:

| Now it's only a 3.3 regression. I'm testing the patch there and waiting for 
| Gaby's approval.

I thought I OKed in a previous message.

Thanks!

-- Gaby
Comment 20 GCC Commits 2004-01-30 18:21:00 UTC
Subject: Bug 13683

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	giovannibajo@gcc.gnu.org	2004-01-30 18:20:55

Modified files:
	gcc/cp         : ChangeLog call.c 

Log message:
	PR c++/13683
	* call.c (convert_arg_to_ellipsis): Don't emit a warning if within
	a sizeof expression.block

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.3076.2.245&r2=1.3076.2.246
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.341.2.36&r2=1.341.2.37

Comment 21 GCC Commits 2004-01-30 18:31:15 UTC
Subject: Bug 13683

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	giovannibajo@gcc.gnu.org	2004-01-30 18:31:08

Modified files:
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/template: sizeof6.C 

Log message:
	PR c++/13683
	* g++.dg/template/sizeof6.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2261.2.353&r2=1.2261.2.354
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/template/sizeof6.C.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=NONE&r2=1.1.4.1

Comment 22 Giovanni Bajo 2004-01-30 18:32:19 UTC
Fixed in 3.3.3 as well. Thank you!