[Bug c++/39242] New: [4.4 Regression] Inconsistent reject / accept of code

[Bug c++/39242] New: [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]

With -O0 the following is accepted, with -O1 it is rejected with

Parser.3.ii: In member function ‘void RepPtrStore<_Tp, _Bt>::_assign(_Tp*)
[with _Tp = YStatement, _Bt = YCode]’:
Parser.3.ii:11:   instantiated from ‘RepPtrStore<_Tp, _Bt>::~RepPtrStore()
[with _Tp = YStatement, _Bt = YCode]’
Parser.3.ii:21:   instantiated from here
Parser.3.ii:16: error: no matching function for call to
‘Rep::unref(YStatement*&)’
Parser.3.ii:3: note: candidates are: void Rep::unref() const
Parser.3.ii:4: note:                 static void Rep::unref(const Rep*)


testcase:

class Rep {
public:
    void unref() const { }
    static void unref (const Rep * obj_r) { obj_r->unref(); }
};
template<typename _Tp, typename _Bt = _Tp>
class RepPtrStore {
    _Tp * _obj;
    void _assign( _Tp * new_r );
public:
    ~RepPtrStore() { _assign( 0 ); }
};
template<typename _Tp,typename _Bt>
void RepPtrStore<_Tp,_Bt>::_assign( _Tp * new_r )
{
  Rep::unref( _obj );
}
class RepPtrBase { };
template<typename _Bt> class PtrBase : public RepPtrBase { };
template<typename _Tp, typename _Bt = _Tp>
class Ptr : public PtrBase<_Bt> {
    RepPtrStore<_Tp,_Bt> _ptr;
};
class YCode;
class YStatement;
typedef Ptr<YStatement,YCode> YStatementPtr;
extern template class RepPtrStore<YStatement,YCode>;
class ExecutionEnvironment {
    YStatementPtr m_statement;
    ~ExecutionEnvironment() { };
};

GCC 4.3 accepts the code unconditionally, so does EDG.


-- 
           Summary: [4.4 Regression] Inconsistent reject / accept of code
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Keywords: rejects-valid
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2009-02-19 13:40 -------
Created an attachment (id=17329)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17329&action=view)
original testcase

Original testcase from Yast YCP.  Requires -std=c++0x.


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.4.0


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2009-02-19 13:46 -------
Randomly poking around the tree shows

Index: cp/ChangeLog
===================================================================
--- cp/ChangeLog        (revision 138149)
+++ cp/ChangeLog        (revision 138150)
@@ -1,3 +1,20 @@
+2008-07-25  Jan Hubicka  <jh@suse.cz>
+
+       * typeck.c (inline_conversion): Remove.
+       (cp_build_function_call): Do not use inline_conversion.
+       * decl.c (duplicate_decls): Do not insist on inline being declared
+       early.
+       (start_cleanup_fn): Do not assume that INLINE flags prevent function
+       from being output.  We now remove static functions always.
+       (finish_function): Do return warning on all static functions.
+       * call.c (build_over_call): Do not use inline_conversion.
+       * cp-tree.h (possibly_inlined_p): Declare.
+       (inline_conversion): Remove.
+       * pt.c (instantiate_decl): Use possibly_inlined_p predicate.
+       * decl2.c (cp_write_global_declarations): Likewise.
+       (mark_used): Likewise.
+       (possibly_inlined_p): New functions.

as very likely candidate introducing the regression (the optimize dependency).


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu dot
                   |                            |org


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2009-02-19 13:50 -------
The error is that RepPtrStore<YStatement,YCode>::assign is instantiated at all
in face of extern template class RepPtrStore<YStatement,YCode>;


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2009-02-19 14:19 -------
This fixes it for me:

Index: decl2.c
===================================================================
--- decl2.c     (revision 144292)
+++ decl2.c     (working copy)
@@ -3755,7 +3755,8 @@ bool
 possibly_inlined_p (tree decl)
 {
   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
-  if (DECL_UNINLINABLE (decl))
+  if (DECL_UNINLINABLE (decl)
+      || DECL_REALLY_EXTERN (decl))
     return false;
   if (!optimize)
     return DECL_DECLARED_INLINE_P (decl);


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2009-02-19 14:30 -------
Or rather we don't want template instantiation (and errors from it) to differ
from optimized to non-optimized build.  So,

Index: cp/pt.c
===================================================================
--- cp/pt.c     (revision 144292)
+++ cp/pt.c     (working copy)
@@ -15285,7 +15285,7 @@ instantiate_decl (tree d, int defer_ok,
       /* ... but we instantiate inline functions so that we can inline
         them and ... */
       && ! (TREE_CODE (d) == FUNCTION_DECL
-           && possibly_inlined_p (d))
+           && DECL_DECLARED_INLINE_P (d))
       /* ... we instantiate static data members whose values are
         needed in integral constant expressions.  */
       && ! (TREE_CODE (d) == VAR_DECL
@@ -15363,7 +15363,7 @@ instantiate_decl (tree d, int defer_ok,
       /* Instantiate inline functions so that the inliner can do its
         job, even though we'll not be emitting a copy of this
         function.  */
-      if (!(TREE_CODE (d) == FUNCTION_DECL && possibly_inlined_p (d)))
+      if (!(TREE_CODE (d) == FUNCTION_DECL && !DECL_UNINLINABLE (d)))
        goto out;
     }

?


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu dot org


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2009-02-19 14:51 -------
Note that in the original testcase YStatement is incomplete, but properly
derives from Rep in the TU with the explicit instantiation of
RepPtrStore<YStatement,YCode>.

Instead of not instantiating so much we could also try to suppress errors
(like SFINAE) here.

Of course reading the C++0x standard about explicit instantiation declarations
isn't very conclusive (either there's no explanation or everything covering
explicit instantiations also covers their declarations).


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at codesourcery dot com


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[hubicka at ucw dot cz]

------- Comment #7 from hubicka at ucw dot cz  2009-02-19 14:59 -------
Subject: Re:  [4.4 Regression] Inconsistent reject / accept of code

> Index: cp/pt.c
> ===================================================================
> --- cp/pt.c     (revision 144292)
> +++ cp/pt.c     (working copy)
> @@ -15285,7 +15285,7 @@ instantiate_decl (tree d, int defer_ok,
>        /* ... but we instantiate inline functions so that we can inline
>          them and ... */
>        && ! (TREE_CODE (d) == FUNCTION_DECL
> -           && possibly_inlined_p (d))
> +           && DECL_DECLARED_INLINE_P (d))

this way we will lose inlining opurtunities at -O3, sine we will never
instantiate methods not declared inline.

Is standard really making difference in between inline and !inline here?
Or we are just supposed to suppress the errors and instantiate only
stuff that is complette or are we supposed to give error at -O0 too or
is it up to our choice?

Honza


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2009-02-19 15:20 -------
Note that the patch only affects extern explicit template instantiations.

  /* Check to see whether we know that this template will be
     instantiated in some other file, as with "extern template"
     extension.  */
  external_p = (DECL_INTERFACE_KNOWN (d) && DECL_REALLY_EXTERN (d));
  /* In general, we do not instantiate such templates...  */
  if (external_p
      /* ... but we instantiate inline functions so that we can inline
         them and ... */
      && ! (TREE_CODE (d) == FUNCTION_DECL
            && DECL_DECLARED_INLINE_P (d))

For them we would no longer inline (or rather instantiate) not inline declared
members.  This may affect parts of libstdc++, but I don't know.

In YCP the extern trick seems to be used to break a dependency cycle, so the
situation is somewhat special.

The ultimate question is of course if the standard allows (or even requires)
an error here.


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[hubicka at ucw dot cz]

------- Comment #9 from hubicka at ucw dot cz  2009-02-19 15:40 -------
Subject: Re:  [4.4 Regression] Inconsistent reject / accept of code

>   /* Check to see whether we know that this template will be
>      instantiated in some other file, as with "extern template"
>      extension.  */
>   external_p = (DECL_INTERFACE_KNOWN (d) && DECL_REALLY_EXTERN (d));
>   /* In general, we do not instantiate such templates...  */
>   if (external_p
>       /* ... but we instantiate inline functions so that we can inline
>          them and ... */
>       && ! (TREE_CODE (d) == FUNCTION_DECL
>             && DECL_DECLARED_INLINE_P (d))

Hmm, in general it is benefical to instantiate stuff for sake of IPA
optimizers.  It would be interesting to know how this affects code
quality...

Honza


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[mark at codesourcery dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2081 bytes --]



------- Comment #10 from mark at codesourcery dot com  2009-02-19 16:41 -------
Subject: Re:  [4.4 Regression] Inconsistent reject / accept
 of code

rguenth at gcc dot gnu dot org wrote:

> The ultimate question is of course if the standard allows (or even requires)
> an error here.

The (someone old) C++ WP I have is pretty clear:

"An explicit instantiation declaration that names a class template
specialization has no effect on the class template specialization
itself (except for perhaps resulting in its implicit instantiation).
Except for inline functions, other explicit
instantiation declarations have the effect of suppressing the implicit
instantiation of the entity to which they refer. [ Note:
The intent is that an inline function that is the subject of an explicit
instantiation declaration will still be implicitly instantiated
when used so that the body can be considered for inlining, but that no
out-of-line copy of the inline function
would be generated in the translation unit. —end note ]"

Here, "inline function" is of course the C++ definition thereof, i.e.,
functions declared "inline" or defined in the body of a class
definition, rather than outside the class.

What that means is that we *must not* implicitly instantiate things
declared "extern template" unless they are DECL_DECLARED_INLINE_P.  As a
consequence, at -O3, we cannot implicitly instantiate non-inline "extern
template" functions.

So, I think first hunk in the patch is correct.  It needs a comment,
though, right above DECL_DECLARED_INLINE to point out that this is a
restriction coming from the standard:

/* An explicit instantiation declaration prohibits implicit
instantiation of non-inline functions.  With high levels of
optimization, we would normally inline non-inline functions -- but we're
not allowed to do that for "extern template" functions.  Therefore, we
check DECL_DECLARED_INLINE_P, rather than possibly_inlined_p.  */

OK with that change.

I don't yet understand why the second hunk is required.


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenther at suse dot de]

------- Comment #11 from rguenther at suse dot de  2009-02-19 16:54 -------
Subject: Re:  [4.4 Regression] Inconsistent reject / accept
 of code

On Thu, 19 Feb 2009, mark at codesourcery dot com wrote:

> ------- Comment #10 from mark at codesourcery dot com  2009-02-19 16:41 -------
> Subject: Re:  [4.4 Regression] Inconsistent reject / accept
>  of code
> 
> rguenth at gcc dot gnu dot org wrote:
> 
> > The ultimate question is of course if the standard allows (or even requires)
> > an error here.
> 
> The (someone old) C++ WP I have is pretty clear:
> 
> "An explicit instantiation declaration that names a class template
> specialization has no effect on the class template specialization
> itself (except for perhaps resulting in its implicit instantiation).
> Except for inline functions, other explicit
> instantiation declarations have the effect of suppressing the implicit
> instantiation of the entity to which they refer. [ Note:
> The intent is that an inline function that is the subject of an explicit
> instantiation declaration will still be implicitly instantiated
> when used so that the body can be considered for inlining, but that no
> out-of-line copy of the inline function
> would be generated in the translation unit. ???end note ]"
> 
> Here, "inline function" is of course the C++ definition thereof, i.e.,
> functions declared "inline" or defined in the body of a class
> definition, rather than outside the class.
> 
> What that means is that we *must not* implicitly instantiate things
> declared "extern template" unless they are DECL_DECLARED_INLINE_P.  As a
> consequence, at -O3, we cannot implicitly instantiate non-inline "extern
> template" functions.
> 
> So, I think first hunk in the patch is correct.  It needs a comment,
> though, right above DECL_DECLARED_INLINE to point out that this is a
> restriction coming from the standard:
> 
> /* An explicit instantiation declaration prohibits implicit
> instantiation of non-inline functions.  With high levels of
> optimization, we would normally inline non-inline functions -- but we're
> not allowed to do that for "extern template" functions.  Therefore, we
> check DECL_DECLARED_INLINE_P, rather than possibly_inlined_p.  */
> 
> OK with that change.
> 
> I don't yet understand why the second hunk is required.

It is probably not, I just looked for all possibly_inlined_p
occurrences in instantiate_decl and tried to make sure behavior
does not depend on optimization level.

I'll split out the first part and test/submit it.

Richard.


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-02-19 16:59:20
               date|                            |


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[jason at gcc dot gnu dot org]

------- Comment #12 from jason at gcc dot gnu dot org  2009-02-19 17:39 -------
(In reply to comment #10)
> What that means is that we *must not* implicitly instantiate things
> declared "extern template" unless they are DECL_DECLARED_INLINE_P.  As a
> consequence, at -O3, we cannot implicitly instantiate non-inline "extern
> template" functions.

I'm not entirely sure that's what we want it to say, but it does seem like a
reasonable expectation for users to have.

Beyond this issue, what is the compile speed impact of the earlier change to
use possibly_inlined_p?  It seems like it might be making us speculatively
instantiate a lot more functions for potential inlining even at -O1, which I
would expect to cause memory bloat and slower compilation.


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[hubicka at ucw dot cz]

------- Comment #13 from hubicka at ucw dot cz  2009-02-20 14:39 -------
Subject: Re:  [4.4 Regression] Inconsistent reject / accept of code

> > What that means is that we *must not* implicitly instantiate things
> > declared "extern template" unless they are DECL_DECLARED_INLINE_P.  As a
> > consequence, at -O3, we cannot implicitly instantiate non-inline "extern
> > template" functions.
> 
> I'm not entirely sure that's what we want it to say, but it does seem like a
> reasonable expectation for users to have.
> 
> Beyond this issue, what is the compile speed impact of the earlier change to
> use possibly_inlined_p?  It seems like it might be making us speculatively
> instantiate a lot more functions for potential inlining even at -O1, which I
> would expect to cause memory bloat and slower compilation.

There was no slowdowns visible on our C++ testers, so I hope it is not
too bad.  We now get rid of unreachable functions quite early in queue
so they are not consuming too much of compilation time.

Honza


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #14 from rguenth at gcc dot gnu dot org  2009-02-21 12:54 -------
Patch posted.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2009-
                   |                            |02/msg00944.html


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #15 from rguenth at gcc dot gnu dot org  2009-02-24 14:50 -------
Subject: Bug 39242

Author: rguenth
Date: Tue Feb 24 14:50:30 2009
New Revision: 144408

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144408
Log:
2009-02-24  Richard Guenther  <rguenther@suse.de>

        PR c++/39242
        * pt.c (instantiate_decl): Do not instantiate extern, non-inline
        declared functions.

        * g++.dg/template/instantiate10.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/template/instantiate10.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/pt.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug c++/39242] [4.4 Regression] Inconsistent reject / accept of code

[rguenth at gcc dot gnu dot org]

------- Comment #16 from rguenth at gcc dot gnu dot org  2009-02-24 14:52 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |4.3.3
         Resolution|                            |FIXED


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