Bug 43333 - [4.5 Regression] __is_pod seems broken
Summary: [4.5 Regression] __is_pod seems broken
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.5.0
: P1 normal
Target Milestone: 4.5.0
Assignee: Jason Merrill
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2010-03-11 16:03 UTC by Michael Matz
Modified: 2010-03-22 20:49 UTC (History)
4 users (show)

See Also:
Host: x86_64-linux
Target:
Build:
Known to work: 4.4.3
Known to fail:
Last reconfirmed: 2010-03-12 02:55:29


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Matz 2010-03-11 16:03:53 UTC
On r157245 (and former revisions) this testcase will abort:
# cat ispod.cc
struct strPOD
{
  const char *const foo;
  const char *const bar;
};
extern "C" void abort (void);
int main ()
{
  if (!__is_pod (strPOD))
    abort ();
  return 0;
}

This manifests itself in blocxx not compiling with gcc 4.5 (due to its use
of tr1::is_pod<> implemented in terms of above).  It still works with a random
gcc 4.3 version.
Comment 1 Richard Biener 2010-03-11 16:24:31 UTC
Confirmed.  4.4 works as well.
Comment 2 Paolo Carlini 2010-03-11 16:41:50 UTC
I would be willing to work on this, of course, but I can't really do it now because I'm traveling and I don't have with me all the tools I need.

Anyway, we do already have a testcase involving a pair of doubles, and I'm surprised that now is broken for a pair of pointers, or the issue it really about the const qualification? In any case, it doesn't look to me as possibly a problem having to do with the implementation of the trait proper, because it boils down to just:

    case CPTK_IS_POD:
      return (pod_type_p (type1));

Maybe Jason can help a bit, I think pod_type_p has been recently changed to deal with std_layout types, etc.
Comment 3 pinskia@gmail.com 2010-03-11 17:01:27 UTC
Subject: Re:   New: __is_pod seems broken



Sent from my iPhone

On Mar 11, 2010, at 8:03 AM, "matz at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org 
 > wrote:

> On r157245 (and former revisions) this testcase will abort:
> # cat ispod.cc
> struct strPOD
> {
>  const char *const foo;
>  const char *const bar;
> };

I don't think this is a pod as it requires a non trivial constructor.



> extern "C" void abort (void);
> int main ()
> {
>  if (!__is_pod (strPOD))
>    abort ();
>  return 0;
> }
>
> This manifests itself in blocxx not compiling with gcc 4.5 (due to  
> its use
> of tr1::is_pod<> implemented in terms of above).  It still works  
> with a random
> gcc 4.3 version.
>
>
> -- 
>           Summary: __is_pod seems broken
>           Product: gcc
>           Version: 4.5.0
>            Status: UNCONFIRMED
>          Severity: normal
>          Priority: P3
>         Component: c++
>        AssignedTo: unassigned at gcc dot gnu dot org
>        ReportedBy: matz at gcc dot gnu dot org
>  GCC host triplet: x86_64-linux
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43333
>
Comment 4 Jonathan Wakely 2010-03-11 17:12:00 UTC
it's both trivial and standard layout, so is a POD
Comment 5 H.J. Lu 2010-03-11 21:12:37 UTC
It is caused by revision 149721:

http://gcc.gnu.org/ml/gcc-cvs/2009-07/msg00602.html
Comment 6 Jason Merrill 2010-03-12 02:48:56 UTC
strPOD isn't trivial; its copy assignment operator is ill-formed/deleted.  This is a change in PODness between C++98 and C++0x which may not have been intended.
Comment 7 Paolo Carlini 2010-03-15 17:23:21 UTC
An additional remark: irrespective of the C++1x PODness, the *TR1* is_pod cannot be broken, because essentially N1836, not requiring compiler support, allows for any behavior outside scalar types (see 4.9/8).
Comment 8 Michael Matz 2010-03-22 16:34:37 UTC
Re comment #6: well, then we still need to fix the c++98 case.
Re comment #7: note carefully how I have avoided is_pod<> in the testcase,
but instead used the internal mean to implement the former.  That's the
regression I'm interested about.  (well, to tell the truth I would also
consider it a regression of is_pod<>, if not by the letter of the standard,
then as quality of implementation, because we _do_ have compiler support)
Comment 9 Paolo Carlini 2010-03-22 16:43:34 UTC
Michael, I'm not sure to follow all the philosophical details of the issue. To be sure:
1- __is_pod implements, to date, the correct C++0x semantics, modulo ISO DRs (probably forthcoming, but a resolution seems quite far)
2- Thus, by definition, __is_pod is the right way to implement std::is_pod.
3- About tr1::is_pod, the issue is a little more muddled, but given my Comment #7, we cannot really do wrong as regards TR1 is concerned (and, frankly I don't care much about TR1). Note that the semantics of tr1::is_pod already changed once, when __is_pod started to be used as an implementation detail.

All in all, if you ask me, until the ISO DR issue is resolved I don't think we should do anything here.
Comment 10 Michael Matz 2010-03-22 16:54:39 UTC
Hmm, well, but there's code out there that expects the "old" TR1 semantic,
namely blocxx, and if the definition is indeed muddled than IMNSHO we should
retain the behaviour as it was in older GCC versions, instead of breaking
existing code.
Comment 11 Paolo Carlini 2010-03-22 17:07:36 UTC
We discussed a bit the issue with Jason in Pittsburgh *before* realizing that likely the C++1x WD is wrong about not categorizing strPOD as POD, which now seems the real issue. My personal point of view is still that all the builtins should reflect, consistently, the ISO C++1x semantics. As an interim solution, until the ISO defect is resolved, we could have an __is_cxx98_pod and an __is_pod. That would be my preference. Jason - again, before realizing that we have a real ISO issue - proposed changing back __is_pod to the c++98 semantics and using __is_trivially_copyable and __is_standard_layout to implement std::is_pod. Frankly, long term, I don't think this is the most consistent and clear solution, in particular to people using the naked builtins, which then would have to learn that *all* the builtins reflect the C++1x semantics *beside* __is_pod. But what can I say, if you really think this is the best interim solution, I can live with it, only let's make **really** sure an ISO issue is opened and resolved quickly, clarifying the mess + let's document those semantics.
Comment 12 Jason Merrill 2010-03-22 20:39:16 UTC
Subject: Bug 43333

Author: jason
Date: Mon Mar 22 20:38:57 2010
New Revision: 157652

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157652
Log:
	PR c++/43333
	* tree.c (pod_type_p): Use old meaning in C++98 mode.

Added:
    trunk/gcc/testsuite/g++.dg/ext/is_pod_98.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/tree.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/ext/is_pod.C

Comment 13 Jason Merrill 2010-03-22 20:49:33 UTC
Fixed by reverting to old semantics in C++98 mode.
Comment 14 hjl@gcc.gnu.org 2010-03-25 16:40:39 UTC
Subject: Bug 43333

Author: hjl
Date: Thu Mar 25 16:39:51 2010
New Revision: 157726

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157726
Log:
Backport regression testcases from mainline.

2010-03-25  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline:
	2010-03-22  Jason Merrill  <jason@redhat.com>

	PR c++/43333
	* g++.dg/ext/is_pod_98.C: New.

	2010-03-22  Michael Matz  <matz@suse.de>

	PR middle-end/43475
	* gfortran.dg/pr43475.f90: New testcase.

	2010-03-22  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43390
	* gfortran.fortran-torture/execute/pr43390.f90: New testcase.

	2010-03-20  Dodji Seketeli  <dodji@redhat.com>

	PR c++/43375
	* g++.dg/abi/mangle42.C: New test.

	2010-03-19  Andrew Pinski  <andrew_pinski@caviumnetworks.com>

	PR C/43211
	* gcc.dg/pr43211.c: New test.

	2010-03-18  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/42450
	* g++.dg/torture/pr42450.C: New test.

	2010-03-18  Michael Matz  <matz@suse.de>

	PR tree-optimization/43402
	* gcc.dg/pr43402.c: New testcase.

	2010-03-17  Peter Bergner  <bergner@vnet.ibm.com>

	PR target/42427
	* gcc.dg/pr42427.c: New test.

	2010-03-16  Richard Guenther  <rguenther@suse.de>

	PR middle-end/43379
	* gcc.dg/pr43379.c: New testcase.

	2010-03-15  Michael Matz  <matz@suse.de>

	PR middle-end/43300
	* gcc.dg/pr43300.c: New testcase.

	2010-03-15  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43367
	* gcc.c-torture/compile/pr43367.c: New testcase.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/abi/mangle42.C
      - copied unchanged from r157725, trunk/gcc/testsuite/g++.dg/abi/mangle42.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/ext/is_pod_98.C
      - copied unchanged from r157725, trunk/gcc/testsuite/g++.dg/ext/is_pod_98.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr42450.C
      - copied unchanged from r157725, trunk/gcc/testsuite/g++.dg/torture/pr42450.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr43367.c
      - copied unchanged from r157725, trunk/gcc/testsuite/gcc.c-torture/compile/pr43367.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr42427.c
      - copied unchanged from r157725, trunk/gcc/testsuite/gcc.dg/pr42427.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr43211.c
      - copied unchanged from r157725, trunk/gcc/testsuite/gcc.dg/pr43211.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr43300.c
      - copied unchanged from r157725, trunk/gcc/testsuite/gcc.dg/pr43300.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr43379.c
      - copied unchanged from r157725, trunk/gcc/testsuite/gcc.dg/pr43379.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr43402.c
      - copied unchanged from r157725, trunk/gcc/testsuite/gcc.dg/pr43402.c
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/pr43475.f90
      - copied unchanged from r157725, trunk/gcc/testsuite/gfortran.dg/pr43475.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.fortran-torture/execute/pr43390.f90
      - copied unchanged from r157725, trunk/gcc/testsuite/gfortran.fortran-torture/execute/pr43390.f90
Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog