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