Re: r258768 - [Sema] Incomplete types are OK for covariant returns
Hi David,
Should this be guarded by if(cxx14)? I think the complete type was required
by earlier standards.
- Kim
Den 26 jan 2016 2:40 fm skrev "David Majnemer via cfe-commits" <
cfe-commits@lists.llvm.org>:
> Author: majnemer
> Date: Mon Jan 25 19:37:01 2016
> New Revision: 258768
>
> URL: http://llvm.org/viewvc/llvm-project?rev=258768&view=rev
> Log:
> [Sema] Incomplete types are OK for covariant returns
>
> Per C++14 [class.virtual]p8, it is OK for the return type's class type
> to be incomplete so long as the return type is the same between the base
> and complete classes.
>
> This fixes PR26297.
>
> Modified:
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/SemaCXX/virtual-override.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=258768&r1=258767&r2=258768&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jan 25 19:37:01 2016
> @@ -13020,19 +13020,20 @@ bool Sema::CheckOverridingFunctionReturn
> return true;
>}
>
> - // C++ [class.virtual]p6:
> - // If the return type of D::f differs from the return type of B::f,
> the
> - // class type in the return type of D::f shall be complete at the
> point of
> - // declaration of D::f or shall be the class type D.
> - if (const RecordType *RT = NewClassTy->getAs()) {
> -if (!RT->isBeingDefined() &&
> -RequireCompleteType(New->getLocation(), NewClassTy,
> -diag::err_covariant_return_incomplete,
> -New->getDeclName()))
> -return true;
> - }
> -
>if (!Context.hasSameUnqualifiedType(NewClassTy, OldClassTy)) {
> +// C++14 [class.virtual]p8:
> +// If the class type in the covariant return type of D::f differs
> from
> +// that of B::f, the class type in the return type of D::f shall be
> +// complete at the point of declaration of D::f or shall be the
> class
> +// type D.
> +if (const RecordType *RT = NewClassTy->getAs()) {
> + if (!RT->isBeingDefined() &&
> + RequireCompleteType(New->getLocation(), NewClassTy,
> + diag::err_covariant_return_incomplete,
> + New->getDeclName()))
> +return true;
> +}
> +
> // Check if the new class derives from the old class.
> if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
>Diag(New->getLocation(), diag::err_covariant_return_not_derived)
>
> Modified: cfe/trunk/test/SemaCXX/virtual-override.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/virtual-override.cpp?rev=258768&r1=258767&r2=258768&view=diff
>
> ==
> --- cfe/trunk/test/SemaCXX/virtual-override.cpp (original)
> +++ cfe/trunk/test/SemaCXX/virtual-override.cpp Mon Jan 25 19:37:01 2016
> @@ -289,3 +289,15 @@ namespace PR8168 {
> static void foo() {} // expected-error{{'static' member function
> 'foo' overrides a virtual function}}
>};
> }
> +
> +namespace PR26297 {
> +struct Incomplete;
> +
> +struct Base {
> + virtual const Incomplete *meow() = 0;
> +};
> +
> +struct Derived : Base {
> + virtual Incomplete *meow() override { return nullptr; }
> +};
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r258768 - [Sema] Incomplete types are OK for covariant returns
On Sunday, January 31, 2016, Kim Gräsman wrote:
> Hi David,
>
> Should this be guarded by if(cxx14)? I think the complete type was
> required by earlier standards.
>
It is the same issue as CWG defect report 1250:
http://wg21.cmeerw.net/cwg/issue1250
I forget how to tell how far back a DR applies but I'd guess this one goes
as far back as C++11.
> - Kim
> Den 26 jan 2016 2:40 fm skrev "David Majnemer via cfe-commits" <
> cfe-commits@lists.llvm.org
> >:
>
>> Author: majnemer
>> Date: Mon Jan 25 19:37:01 2016
>> New Revision: 258768
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=258768&view=rev
>> Log:
>> [Sema] Incomplete types are OK for covariant returns
>>
>> Per C++14 [class.virtual]p8, it is OK for the return type's class type
>> to be incomplete so long as the return type is the same between the base
>> and complete classes.
>>
>> This fixes PR26297.
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/test/SemaCXX/virtual-override.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=258768&r1=258767&r2=258768&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jan 25 19:37:01 2016
>> @@ -13020,19 +13020,20 @@ bool Sema::CheckOverridingFunctionReturn
>> return true;
>>}
>>
>> - // C++ [class.virtual]p6:
>> - // If the return type of D::f differs from the return type of B::f,
>> the
>> - // class type in the return type of D::f shall be complete at the
>> point of
>> - // declaration of D::f or shall be the class type D.
>> - if (const RecordType *RT = NewClassTy->getAs()) {
>> -if (!RT->isBeingDefined() &&
>> -RequireCompleteType(New->getLocation(), NewClassTy,
>> -diag::err_covariant_return_incomplete,
>> -New->getDeclName()))
>> -return true;
>> - }
>> -
>>if (!Context.hasSameUnqualifiedType(NewClassTy, OldClassTy)) {
>> +// C++14 [class.virtual]p8:
>> +// If the class type in the covariant return type of D::f differs
>> from
>> +// that of B::f, the class type in the return type of D::f shall be
>> +// complete at the point of declaration of D::f or shall be the
>> class
>> +// type D.
>> +if (const RecordType *RT = NewClassTy->getAs()) {
>> + if (!RT->isBeingDefined() &&
>> + RequireCompleteType(New->getLocation(), NewClassTy,
>> + diag::err_covariant_return_incomplete,
>> + New->getDeclName()))
>> +return true;
>> +}
>> +
>> // Check if the new class derives from the old class.
>> if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
>>Diag(New->getLocation(), diag::err_covariant_return_not_derived)
>>
>> Modified: cfe/trunk/test/SemaCXX/virtual-override.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/virtual-override.cpp?rev=258768&r1=258767&r2=258768&view=diff
>>
>> ==
>> --- cfe/trunk/test/SemaCXX/virtual-override.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/virtual-override.cpp Mon Jan 25 19:37:01 2016
>> @@ -289,3 +289,15 @@ namespace PR8168 {
>> static void foo() {} // expected-error{{'static' member function
>> 'foo' overrides a virtual function}}
>>};
>> }
>> +
>> +namespace PR26297 {
>> +struct Incomplete;
>> +
>> +struct Base {
>> + virtual const Incomplete *meow() = 0;
>> +};
>> +
>> +struct Derived : Base {
>> + virtual Incomplete *meow() override { return nullptr; }
>> +};
>> +}
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r258768 - [Sema] Incomplete types are OK for covariant returns
On Sun, Jan 31, 2016 at 2:50 PM, David Majnemer wrote: > > It is the same issue as CWG defect report 1250: > http://wg21.cmeerw.net/cwg/issue1250 > > I forget how to tell how far back a DR applies but I'd guess this one goes > as far back as C++11. Thanks, I didn't know there was a DR for this. I find the standards text is a little vague but your change seems to indicate that cv-qualifiers never weighs into covariance. The reason I ask is we handle this explicitly in IWYU, and we treat cv-qual differences as covariant (requiring the complete type). It doesn't look like that was the standard's intent. We should probably follow Clang here in a coming release, assuming other compilers agree. It looks from the bug-report that they do, at least newer versions. Thanks, - Kim ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r258768 - [Sema] Incomplete types are OK for covariant returns
On Sun, Jan 31, 2016 at 5:24 PM, Kim Gräsman wrote: > On Sun, Jan 31, 2016 at 2:50 PM, David Majnemer > wrote: >> >> It is the same issue as CWG defect report 1250: >> http://wg21.cmeerw.net/cwg/issue1250 >> >> I forget how to tell how far back a DR applies but I'd guess this one goes >> as far back as C++11. > > Thanks, I didn't know there was a DR for this. Oh, this should probably be updated: http://clang.llvm.org/cxx_dr_status.html#1250 - K ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r258768 - [Sema] Incomplete types are OK for covariant returns
Done in r258768. On Sun, Jan 31, 2016 at 10:49 AM, Kim Gräsman wrote: > On Sun, Jan 31, 2016 at 5:24 PM, Kim Gräsman > wrote: > > On Sun, Jan 31, 2016 at 2:50 PM, David Majnemer > > wrote: > >> > >> It is the same issue as CWG defect report 1250: > >> http://wg21.cmeerw.net/cwg/issue1250 > >> > >> I forget how to tell how far back a DR applies but I'd guess this one > goes > >> as far back as C++11. > > > > Thanks, I didn't know there was a DR for this. > > Oh, this should probably be updated: > http://clang.llvm.org/cxx_dr_status.html#1250 > > - K > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
