<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jan 10, 2014 at 10:30 AM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:kyrtzidis@apple.com" target="_blank">kyrtzidis@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div class="im">On Jan 9, 2014, at 3:21 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
</div><div><div class="im"><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jan 9, 2014 at 3:08 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:kyrtzidis@apple.com" target="_blank">kyrtzidis@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Richard,<br>
<br>
This caused rejecting the following valid code, could you take a look ?<br>
<br>
$ cat t.cpp<br>
struct S {<br>
int x;<br>
};<br>
struct Info {<br>
volatile S d;<br>
};<br>
struct RO {<br>
Info info[];<br>
};<br>
<br>
$ clang -fsyntax-only -std=c++11 t.cpp<br>
t.cpp:9:8: error: flexible array member 'info' of non-POD element type 'Info []'<br>
Info info[];<br>
^<br>
1 error generated.</blockquote><div><br></div><div>The text in this diagnostic looks correct; Info is non-POD in C++11 because it's not trivially copyable due to the volatile member.</div></div></div></div></blockquote>
<div><br></div></div><div>Is there a snippet from the C++11 standard related to your change that you could add ?</div></div></div></blockquote><div><br></div><div>Here's how it plays out:</div><div><br></div><div>3.9/9: "[...], POD classes, [...] are collectively called POD types"</div>
<div>9/10: "A POD class is a class that is either a POD struct or a POD union"<br></div><div>9/10: "A POD struct is a non-union class that is both a trivial class and a standard-layout class [...]"<br>
</div><div>9/6: "A trivial class is a class that [...] is trivially copyable"<br></div><div>9/6: "A trivially copyable class is a class that [...] has no non-trivial copy constructors"<br></div><div>From <a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#496">http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#496</a>: "A copy/move constructor for class X is trivial if it is not user-provided, its declared parameter type is the same as if it had been implicitly declared, and if [...] class X has no non-static data members of volatile-qualified type"<br>
</div><div><br></div><div>Note that flexible arrays in C++ are a GNU extension anyway, so this isn't really governed by *any* standard.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div class="im"><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>However, the rule we apply here seems significantly too restrictive; we should probably instead require only that flexible array members have a trivial default constructor and a trivial destructor.</div>
</div></div></div></blockquote><div><br></div></div><div>So.. are you interested in fixing this ? ;-)</div></div></div></blockquote><div><br></div><div>I've pared the check back to just checking whether the flexible array element is trivially destructible in r198983. I suspect we could remove it entirely, but that might limit our flexibility in how we handle this extension later. GCC has no such restriction...</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>
<div class="h5"><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>
On Nov 24, 2013, at 11:07 PM, Richard Smith <<a href="mailto:richard-llvm@metafoo.co.uk" target="_blank">richard-llvm@metafoo.co.uk</a>> wrote:<br>
<br>
> Author: rsmith<br>
> Date: Mon Nov 25 01:07:05 2013<br>
> New Revision: 195620<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=195620&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=195620&view=rev</a><br>
> Log:<br>
> Take cv-qualifiers on fields of class type into account when determining<br>
> whether a defaulted special member function should be deleted.<br>
><br>
> Modified:<br>
> cfe/trunk/lib/AST/DeclCXX.cpp<br>
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
> cfe/trunk/test/CXX/except/except.spec/p14.cpp<br>
> cfe/trunk/test/CXX/special/class.copy/p11.0x.copy.cpp<br>
> cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp<br>
> cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp<br>
><br>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=195620&r1=195619&r2=195620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=195620&r1=195619&r2=195620&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)<br>
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Nov 25 01:07:05 2013<br>
> @@ -722,6 +722,13 @@ void CXXRecordDecl::addedMember(Decl *D)<br>
> if (FieldRec->getDefinition()) {<br>
> addedClassSubobject(FieldRec);<br>
><br>
> + // We may need to perform overload resolution to determine whether a<br>
> + // field can be moved if it's const or volatile qualified.<br>
> + if (T.getCVRQualifiers() & (Qualifiers::Const | Qualifiers::Volatile)) {<br>
> + data().NeedOverloadResolutionForMoveConstructor = true;<br>
> + data().NeedOverloadResolutionForMoveAssignment = true;<br>
> + }<br>
> +<br>
> // C++11 [class.ctor]p5, C++11 [class.copy]p11:<br>
> // A defaulted [special member] for a class X is defined as<br>
> // deleted if:<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=195620&r1=195619&r2=195620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=195620&r1=195619&r2=195620&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Nov 25 01:07:05 2013<br>
> @@ -4913,6 +4913,10 @@ struct SpecialMemberDeletionInfo {<br>
> // cv-qualifiers on class members don't affect default ctor / dtor calls.<br>
> if (CSM == Sema::CXXDefaultConstructor || CSM == Sema::CXXDestructor)<br>
> Quals = 0;<br>
> + // cv-qualifiers on class members affect the type of both '*this' and the<br>
> + // argument for an assignment.<br>
> + if (IsAssignment)<br>
> + TQ |= Quals;<br>
> return S.LookupSpecialMember(Class, CSM,<br>
> ConstArg || (Quals & Qualifiers::Const),<br>
> VolatileArg || (Quals & Qualifiers::Volatile),<br>
><br>
> Modified: cfe/trunk/test/CXX/except/except.spec/p14.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p14.cpp?rev=195620&r1=195619&r2=195620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p14.cpp?rev=195620&r1=195619&r2=195620&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CXX/except/except.spec/p14.cpp (original)<br>
> +++ cfe/trunk/test/CXX/except/except.spec/p14.cpp Mon Nov 25 01:07:05 2013<br>
> @@ -44,8 +44,8 @@ namespace PR13381 {<br>
> struct NoThrowMove {<br>
> NoThrowMove(const NoThrowMove &);<br>
> NoThrowMove(NoThrowMove &&) noexcept;<br>
> - NoThrowMove &operator=(const NoThrowMove &);<br>
> - NoThrowMove &operator=(NoThrowMove &&) noexcept;<br>
> + NoThrowMove &operator=(const NoThrowMove &) const;<br>
> + NoThrowMove &operator=(NoThrowMove &&) const noexcept;<br>
> };<br>
> struct NoThrowMoveOnly {<br>
> NoThrowMoveOnly(NoThrowMoveOnly &&) noexcept;<br>
><br>
> Modified: cfe/trunk/test/CXX/special/class.copy/p11.0x.copy.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p11.0x.copy.cpp?rev=195620&r1=195619&r2=195620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p11.0x.copy.cpp?rev=195620&r1=195619&r2=195620&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CXX/special/class.copy/p11.0x.copy.cpp (original)<br>
> +++ cfe/trunk/test/CXX/special/class.copy/p11.0x.copy.cpp Mon Nov 25 01:07:05 2013<br>
> @@ -1,5 +1,6 @@<br>
> // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s<br>
><br>
> +struct Trivial {};<br>
> struct NonTrivial {<br>
> NonTrivial(const NonTrivial&);<br>
> };<br>
> @@ -69,6 +70,12 @@ struct Deleted {<br>
> Deleted Da;<br>
> Deleted Db(Da); // expected-error{{call to implicitly-deleted copy constructor}}<br>
><br>
> +// It's implied (but not stated) that this also applies in the case where<br>
> +// overload resolution would fail.<br>
> +struct VolatileMember {<br>
> + volatile Trivial vm; // expected-note {{has no copy}}<br>
> +} vm1, vm2(vm1); // expected-error {{deleted}}<br>
> +<br>
> // -- a direct or virtual base class B that cannot be copied because overload<br>
> // resolution results in an ambiguity or a function that is deleted or<br>
> // inaccessible<br>
><br>
> Modified: cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp?rev=195620&r1=195619&r2=195620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp?rev=195620&r1=195619&r2=195620&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp (original)<br>
> +++ cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp Mon Nov 25 01:07:05 2013<br>
> @@ -1,5 +1,6 @@<br>
> // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s<br>
><br>
> +struct Trivial {};<br>
> struct NonTrivial {<br>
> NonTrivial(NonTrivial&&);<br>
> };<br>
> @@ -61,6 +62,24 @@ struct Deleted {<br>
> };<br>
> Deleted::Deleted(Deleted&&) = default; // expected-error{{would delete}}<br>
><br>
> +// It's implied (but not stated) that this should also happen if overload<br>
> +// resolution fails.<br>
> +struct ConstMember {<br>
> + const Trivial ct;<br>
> + ConstMember(ConstMember&&);<br>
> +};<br>
> +ConstMember::ConstMember(ConstMember&&) = default; // ok, calls copy ctor<br>
> +struct ConstMoveOnlyMember {<br>
> + const NonTrivial cnt;<br>
> + ConstMoveOnlyMember(ConstMoveOnlyMember&&);<br>
> +};<br>
> +ConstMoveOnlyMember::ConstMoveOnlyMember(ConstMoveOnlyMember&&) = default; // expected-error{{would delete}}<br>
> +struct VolatileMember {<br>
> + volatile Trivial vt;<br>
> + VolatileMember(VolatileMember&&);<br>
> +};<br>
> +VolatileMember::VolatileMember(VolatileMember&&) = default; // expected-error{{would delete}}<br>
> +<br>
> // -- a direct or virtual base class B that cannot be moved because overload<br>
> // resolution results in an ambiguity or a function that is deleted or<br>
> // inaccessible<br>
><br>
> Modified: cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp?rev=195620&r1=195619&r2=195620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp?rev=195620&r1=195619&r2=195620&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp (original)<br>
> +++ cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp Mon Nov 25 01:07:05 2013<br>
> @@ -1,5 +1,7 @@<br>
> // RUN: %clang_cc1 -verify %s -std=c++11<br>
><br>
> +struct Trivial {};<br>
> +<br>
> template<typename T> struct CopyAssign {<br>
> static T t;<br>
> void test() {<br>
> @@ -47,6 +49,9 @@ class InaccessibleCopyAssign {<br>
> class InaccessibleMoveAssign {<br>
> InaccessibleMoveAssign &operator=(InaccessibleMoveAssign &&);<br>
> };<br>
> +class NonConstCopyAssign {<br>
> + NonConstCopyAssign &operator=(NonConstCopyAssign &);<br>
> +};<br>
><br>
> // A defaulted copy/move assignment operator for class X is defined as deleted<br>
> // if X has:<br>
> @@ -114,6 +119,12 @@ struct D6 {<br>
> D6 &operator=(D6 &&) = default; // expected-note {{here}} expected-note {{copy assignment operator is implicitly deleted}}<br>
> InaccessibleMoveAssign a; // expected-note {{field 'a' has an inaccessible move}}<br>
> };<br>
> +struct D7 {<br>
> + const Trivial a; // expected-note 3{{field 'a' has no }}<br>
> +};<br>
> +struct D8 {<br>
> + volatile Trivial a; // expected-note 3{{field 'a' has no }}<br>
> +};<br>
> template struct CopyAssign<D1>; // expected-note {{here}}<br>
> template struct MoveAssign<D2>; // expected-note {{here}}<br>
> template struct MoveOrCopyAssign<D2>; // expected-note {{here}}<br>
> @@ -123,6 +134,12 @@ template struct MoveOrCopyAssign<D4>; //<br>
> template struct CopyAssign<D5>; // expected-note {{here}}<br>
> template struct MoveAssign<D6>; // expected-note {{here}}<br>
> template struct MoveOrCopyAssign<D6>; // expected-note {{here}}<br>
> +template struct CopyAssign<D7>; // expected-note {{here}}<br>
> +template struct MoveAssign<D7>; // expected-note {{here}}<br>
> +template struct MoveOrCopyAssign<D7>; // expected-note {{here}}<br>
> +template struct CopyAssign<D8>; // expected-note {{here}}<br>
> +template struct MoveAssign<D8>; // expected-note {{here}}<br>
> +template struct MoveOrCopyAssign<D8>; // expected-note {{here}}<br>
><br>
> // -- a direct or virtual base that cannot be copied/moved<br>
> struct E1 : AmbiguousCopyAssign {}; // expected-note {{base class 'AmbiguousCopyAssign' has multiple copy}}<br>
> @@ -147,7 +164,7 @@ template struct MoveAssign<E6>; // expec<br>
> namespace PR13381 {<br>
> struct S {<br>
> S &operator=(const S&);<br>
> - S &operator=(const volatile S&) = delete; // expected-note{{deleted here}}<br>
> + S &operator=(const volatile S&) volatile = delete; // expected-note{{deleted here}}<br>
> };<br>
> struct T {<br>
> volatile S s; // expected-note{{field 's' has a deleted copy assignment}}<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>
</blockquote></div></div></div><br></div>
</blockquote></div><br></div></div>