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