[patch] c++/58051 Implement Core 1579

[patch] c++/58051 Implement Core 1579

[Jonathan Wakely]

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

DR1579 relaxes [class.copy]/32 so that expressions in return
statements can be looked up as rvalues even when they aren't the same
type as the function return type.

Implementing that seems as simple as removing the restriction on the
types. Tested x86_64-linux, no regressions.

OK for trunk?


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1743 bytes --]

commit 45e8a7ceb267cafde4d4411563a3e84bbd49ad8c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jun 26 11:00:54 2014 +0100

    gcc/cp:
    	DR 1579
    	PR c++/58051
    	* typeck.c (check_return_expr): Lookup as an rvalue even when the
    	types aren't the same.
    
    gcc/testsuite:
    	* g++.dg/cpp0x/elision_conv.C: New.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 65dccf7..042e600 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -8607,7 +8607,7 @@ check_return_expr (tree retval, bool *no_warning)
       if (VOID_TYPE_P (functype))
 	return error_mark_node;
 
-      /* Under C++0x [12.8/16 class.copy], a returned lvalue is sometimes
+      /* Under C++11 [12.8/32 class.copy], a returned lvalue is sometimes
 	 treated as an rvalue for the purposes of overload resolution to
 	 favor move constructors over copy constructors.
 
@@ -8618,8 +8618,6 @@ check_return_expr (tree retval, bool *no_warning)
 	      || TREE_CODE (retval) == PARM_DECL)
 	  && DECL_CONTEXT (retval) == current_function_decl
 	  && !TREE_STATIC (retval)
-	  && same_type_p ((TYPE_MAIN_VARIANT (TREE_TYPE (retval))),
-			  (TYPE_MAIN_VARIANT (functype)))
 	  /* This is only interesting for class type.  */
 	  && CLASS_TYPE_P (functype))
 	flags = flags | LOOKUP_PREFER_RVALUE;
diff --git a/gcc/testsuite/g++.dg/cpp0x/elision_conv.C b/gcc/testsuite/g++.dg/cpp0x/elision_conv.C
new file mode 100644
index 0000000..d778a0b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/elision_conv.C
@@ -0,0 +1,18 @@
+// Core 1579 return by converting move constructor
+// PR c++/58051
+// { dg-do compile { target c++11 } }
+
+struct A {
+  A() = default;
+  A(A&&) = default;
+};
+
+struct B {
+  B(A) { }
+};
+
+B f()
+{
+  A a;
+  return a;
+}

Re: [patch] c++/58051 Implement Core 1579

[Jason Merrill]

OK.

Jason

Re: [patch] c++/58051 Implement Core 1579

[Markus Trippelsdorf]

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

On 2014.06.26 at 14:06 +0100, Jonathan Wakely wrote:
> DR1579 relaxes [class.copy]/32 so that expressions in return
> statements can be looked up as rvalues even when they aren't the same
> type as the function return type.
> 
> Implementing that seems as simple as removing the restriction on the
> types. Tested x86_64-linux, no regressions.

This patch cause yet another LLVM build error:

FAILED: /var/tmp/gcc_test/usr/local/bin/g++   -DCLANG_ENABLE_ARCMT -DCLANG_ENABLE_REWRITER -DCLANG_ENABLE_STATIC_ANALYZER -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DclangFrontend_EXPORTS -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wnon-virtual-dtor -Wno-comment -std=c++11 -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wcast-qual -fno-strict-aliasing -O2  -DNDEBUG -pipe -fPIC -Itools/clang/lib/Frontend -I/var/tmp/llvm-project/llvm/tools/clang/lib/Frontend -I/var/tmp/llvm-project/llvm/tools/clang/include -Itools/clang/include -Iinclude -I/var/tmp/llvm-project/llvm/include    -fno-exceptions -fno-rtti -MMD -MT tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o -MF "tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o.d" -o tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o -c /var/tmp/llvm-project/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp
In file included from /var/tmp/llvm-project/llvm/tools/clang/include/clang/Basic/DiagnosticOptions.h:14:0,
                 from /var/tmp/llvm-project/llvm/tools/clang/include/clang/Frontend/CompilerInvocation.h:13,
                 from /var/tmp/llvm-project/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp:10:
/var/tmp/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h: In instantiation of ‘llvm::IntrusiveRefCntPtr<T>::IntrusiveRefCntPtr(llvm::IntrusiveRefCntPtr<X>&&) [with X = clang::vfs::OverlayFileSystem; T = clang::vfs::FileSystem]’:
/var/tmp/llvm-project/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp:2047:10:   required from here
/var/tmp/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:137:8: error: ‘clang::vfs::OverlayFileSystem* llvm::IntrusiveRefCntPtr<clang::vfs::OverlayFileSystem>::Obj’ is private
     T* Obj;
        ^
/var/tmp/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:158:13: error: within this context
       S.Obj = 0;
             ^


Reduced:

markus@x4 llvm_build % cat CompilerInvocation.ii
template <typename T> class A
{
  T Obj;

public:
  T element_type;
  A (T *);
  template <class X> A (A<X> &&p1) { p1.Obj; }
  template <class X> A (A<X> &);
};

class B
{
public:
  B (A<int>);
};
A<int> fn1 ()
{
  A<B> a (new B (0));
  return a;
}

markus@x4 llvm_build % /var/tmp/gcc_test/usr/local/bin/g++ -c -std=c++11 CompilerInvocation.ii
CompilerInvocation.ii: In instantiation of ‘A<T>::A(A<X>&&) [with X = B; T = int]’:
CompilerInvocation.ii:20:10:   required from here
CompilerInvocation.ii:3:5: error: ‘B A<B>::Obj’ is private
   T Obj;
     ^
CompilerInvocation.ii:8:38: error: within this context
   template <class X> A (A<X> &&p1) { p1.Obj; }
                                      ^

-- 
Markus

[-- Attachment #2: CompilerInvocation.ii --]
[-- Type: text/plain, Size: 250 bytes --]

template <typename T> class A
{
  T Obj;

public:
  T element_type;
  A (T *);
  template <class X> A (A<X> &&p1) { p1.Obj; }
  template <class X> A (A<X> &);
};

class B
{
public:
  B (A<int>);
};
A<int> fn1 ()
{
  A<B> a (new B (0));
  return a;
}

Re: [patch] c++/58051 Implement Core 1579

[Marc Glisse]

On Tue, 1 Jul 2014, Markus Trippelsdorf wrote:

> This patch cause yet another LLVM build error:
[...]
> Reduced:
>
> markus@x4 llvm_build % cat CompilerInvocation.ii
> template <typename T> class A
> {
>  T Obj;
>
> public:
>  T element_type;
>  A (T *);
>  template <class X> A (A<X> &&p1) { p1.Obj; }
>  template <class X> A (A<X> &);
> };
>
> class B
> {
> public:
>  B (A<int>);
> };
> A<int> fn1 ()
> {
>  A<B> a (new B (0));
>  return a;
> }
>
> markus@x4 llvm_build % /var/tmp/gcc_test/usr/local/bin/g++ -c -std=c++11 CompilerInvocation.ii
> CompilerInvocation.ii: In instantiation of Β‘A<T>::A(A<X>&&) [with X = B; T = int]Β’:
> CompilerInvocation.ii:20:10:   required from here
> CompilerInvocation.ii:3:5: error: Β‘B A<B>::ObjΒ’ is private
>   T Obj;
>     ^
> CompilerInvocation.ii:8:38: error: within this context
>   template <class X> A (A<X> &&p1) { p1.Obj; }
>                                      ^

This looks invalid. As the core issue says, the return statement looks up 
a as an rvalue, so it uses that constructor for A<int>, and that uses a 
private member of a different specialization of A, which is illegal.

-- 
Marc Glisse

Re: [patch] c++/58051 Implement Core 1579

[Jonathan Wakely]

On 01/07/14 15:06 +0200, Markus Trippelsdorf wrote:
>On 2014.06.26 at 14:06 +0100, Jonathan Wakely wrote:
>> DR1579 relaxes [class.copy]/32 so that expressions in return
>> statements can be looked up as rvalues even when they aren't the same
>> type as the function return type.
>>
>> Implementing that seems as simple as removing the restriction on the
>> types. Tested x86_64-linux, no regressions.
>
>This patch cause yet another LLVM build error:
>
>FAILED: /var/tmp/gcc_test/usr/local/bin/g++   -DCLANG_ENABLE_ARCMT -DCLANG_ENABLE_REWRITER -DCLANG_ENABLE_STATIC_ANALYZER -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DclangFrontend_EXPORTS -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wnon-virtual-dtor -Wno-comment -std=c++11 -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wcast-qual -fno-strict-aliasing -O2  -DNDEBUG -pipe -fPIC -Itools/clang/lib/Frontend -I/var/tmp/llvm-project/llvm/tools/clang/lib/Frontend -I/var/tmp/llvm-project/llvm/tools/clang/include -Itools/clang/include -Iinclude -I/var/tmp/llvm-project/llvm/include    -fno-exceptions -fno-rtti -MMD -MT tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o -MF "tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o.d" -o tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o -c /var/tmp/llvm-project/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp
>In file included from /var/tmp/llvm-project/llvm/tools/clang/include/clang/Basic/DiagnosticOptions.h:14:0,
>                 from /var/tmp/llvm-project/llvm/tools/clang/include/clang/Frontend/CompilerInvocation.h:13,
>                 from /var/tmp/llvm-project/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp:10:
>/var/tmp/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h: In instantiation of ‘llvm::IntrusiveRefCntPtr<T>::IntrusiveRefCntPtr(llvm::IntrusiveRefCntPtr<X>&&) [with X = clang::vfs::OverlayFileSystem; T = clang::vfs::FileSystem]’:
>/var/tmp/llvm-project/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp:2047:10:   required from here
>/var/tmp/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:137:8: error: ‘clang::vfs::OverlayFileSystem* llvm::IntrusiveRefCntPtr<clang::vfs::OverlayFileSystem>::Obj’ is private
>     T* Obj;
>        ^
>/var/tmp/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:158:13: error: within this context
>       S.Obj = 0;
>             ^
>
>
>Reduced:
>
>markus@x4 llvm_build % cat CompilerInvocation.ii
>template <typename T> class A
>{
>  T Obj;
>
>public:
>  T element_type;
>  A (T *);
>  template <class X> A (A<X> &&p1) { p1.Obj; }
>  template <class X> A (A<X> &);
>};
>
>class B
>{
>public:
>  B (A<int>);
>};
>A<int> fn1 ()
>{
>  A<B> a (new B (0));
>  return a;
>}
>
>markus@x4 llvm_build % /var/tmp/gcc_test/usr/local/bin/g++ -c -std=c++11 CompilerInvocation.ii
>CompilerInvocation.ii: In instantiation of ‘A<T>::A(A<X>&&) [with X = B; T = int]’:
>CompilerInvocation.ii:20:10:   required from here
>CompilerInvocation.ii:3:5: error: ‘B A<B>::Obj’ is private
>   T Obj;
>     ^
>CompilerInvocation.ii:8:38: error: within this context
>   template <class X> A (A<X> &&p1) { p1.Obj; }

The error looks correct, A<T> cannot access private members of A<X>.

My patch only affects return statements and you get exactly the same
error if you change the code so there's no return statement:

void fn2 ()
{
  A<B> a (new B (0));
  A<int> aa( static_cast<A<B>&&>(a) );
}

Re: [patch] c++/58051 Implement Core 1579

[Jonathan Wakely]

On 01/07/14 16:10 +0200, Marc Glisse wrote:
>On Tue, 1 Jul 2014, Markus Trippelsdorf wrote:
>
>>This patch cause yet another LLVM build error:
>[...]
>>Reduced:
>>
>>markus@x4 llvm_build % cat CompilerInvocation.ii
>>template <typename T> class A
>>{
>> T Obj;
>>
>>public:
>> T element_type;
>> A (T *);
>> template <class X> A (A<X> &&p1) { p1.Obj; }
>> template <class X> A (A<X> &);
>>};
>>
>>class B
>>{
>>public:
>> B (A<int>);
>>};
>>A<int> fn1 ()
>>{
>> A<B> a (new B (0));
>> return a;
>>}
>>
>>markus@x4 llvm_build % /var/tmp/gcc_test/usr/local/bin/g++ -c -std=c++11 CompilerInvocation.ii
>>CompilerInvocation.ii: In instantiation of ‘A<T>::A(A<X>&&) [with X = B; T = int]’:
>>CompilerInvocation.ii:20:10:   required from here
>>CompilerInvocation.ii:3:5: error: ‘B A<B>::Obj’ is private
>>  T Obj;
>>    ^
>>CompilerInvocation.ii:8:38: error: within this context
>>  template <class X> A (A<X> &&p1) { p1.Obj; }
>>                                     ^
>
>This looks invalid. As the core issue says, the return statement looks 
>up a as an rvalue, so it uses that constructor for A<int>, and that 
>uses a private member of a different specialization of A, which is 
>illegal.

Right, it looks as though that constructor has never been compiled or
tested before, so GCC is not only making the code faster but also
finding a bug :-)

The most obvious fix is to add:

  template<class X> friend class IntrusiveRefCntPtr<X>;

so that conversion from rvalues of different types is supported.

Re: [patch] c++/58051 Implement Core 1579

[Marc Glisse]

On Tue, 1 Jul 2014, Jonathan Wakely wrote:

> Right, it looks as though that constructor has never been compiled or
> tested before, so GCC is not only making the code faster but also
> finding a bug :-)
>
> The most obvious fix is to add:
>
> template<class X> friend class IntrusiveRefCntPtr<X>;
>
> so that conversion from rvalues of different types is supported.

No <X> in the friend declaration though.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=12429

-- 
Marc Glisse