r195620 - Take cv-qualifiers on fields of class type into account when determining
Richard Smith
richard at metafoo.co.uk
Fri Jan 10 17:00:50 PST 2014
On Fri, Jan 10, 2014 at 10:30 AM, Argyrios Kyrtzidis <kyrtzidis at apple.com>wrote:
> On Jan 9, 2014, at 3:21 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Thu, Jan 9, 2014 at 3:08 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com>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 <richard-llvm at metafoo.co.uk>
>> 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
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140110/cd8d8d23/attachment.html>
More information about the cfe-commits
mailing list