| Message ID | 20140626130621.GJ2711@redhat.com |
|---|---|
| State | New |
| Headers | show |
OK. Jason
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; } ^
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.
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) ); }
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.
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
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;
+}