[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: r258768 - [Sema] Incomplete types are OK for covariant returns
From: Kim Gräsman via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2016-01-31 12:24:17
Message-ID: CANt7B+cb3ejCQF9Wg4qFK-nudEk8dKR_6-8UeGVJBztrJusN4w () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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<RecordType>()) {
> - 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<RecordType>()) {
> + 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
>
[Attachment #5 (text/html)]
<p dir="ltr">Hi David, </p>
<p dir="ltr">Should this be guarded by if(cxx14)? I think the complete type was \
required by earlier standards.</p> <p dir="ltr">- Kim </p>
<div class="gmail_quote">Den 26 jan 2016 2:40 fm skrev "David Majnemer via \
cfe-commits" <<a \
href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>>:<br \
type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">Author: \
majnemer<br>
Date: Mon Jan 25 19:37:01 2016<br>
New Revision: 258768<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=258768&view=rev" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=258768&view=rev</a><br>
Log:<br>
[Sema] Incomplete types are OK for covariant returns<br>
<br>
Per C++14 [class.virtual]p8, it is OK for the return type's class type<br>
to be incomplete so long as the return type is the same between the base<br>
and complete classes.<br>
<br>
This fixes PR26297.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
cfe/trunk/test/SemaCXX/virtual-override.cpp<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=258768&r1=258767&r2=258768&view=diff" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=258768&r1=258767&r2=258768&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jan 25 19:37:01 2016<br>
@@ -13020,19 +13020,20 @@ bool Sema::CheckOverridingFunctionReturn<br>
return true;<br>
}<br>
<br>
- // C++ [class.virtual]p6:<br>
- // If the return type of D::f differs from the return type of B::f, the<br>
- // class type in the return type of D::f shall be complete at the point \
of<br>
- // declaration of D::f or shall be the class type D.<br>
- if (const RecordType *RT = NewClassTy->getAs<RecordType>()) {<br>
- if (!RT->isBeingDefined() &&<br>
- RequireCompleteType(New->getLocation(), NewClassTy,<br>
- diag::err_covariant_return_incomplete,<br>
- New->getDeclName()))<br>
- return true;<br>
- }<br>
-<br>
if (!Context.hasSameUnqualifiedType(NewClassTy, OldClassTy)) {<br>
+ // C++14 [class.virtual]p8:<br>
+ // If the class type in the covariant return type of D::f differs from<br>
+ // that of B::f, the class type in the return type of D::f shall be<br>
+ // complete at the point of declaration of D::f or shall be the class<br>
+ // type D.<br>
+ if (const RecordType *RT = NewClassTy->getAs<RecordType>()) {<br>
+ if (!RT->isBeingDefined() &&<br>
+ RequireCompleteType(New->getLocation(), NewClassTy,<br>
+ \
diag::err_covariant_return_incomplete,<br> + \
New->getDeclName()))<br> + return true;<br>
+ }<br>
+<br>
// Check if the new class derives from the old class.<br>
if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {<br>
Diag(New->getLocation(), diag::err_covariant_return_not_derived)<br>
<br>
Modified: cfe/trunk/test/SemaCXX/virtual-override.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/virtual-override.cpp?rev=258768&r1=258767&r2=258768&view=diff" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Se \
maCXX/virtual-override.cpp?rev=258768&r1=258767&r2=258768&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/virtual-override.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/virtual-override.cpp Mon Jan 25 19:37:01 2016<br>
@@ -289,3 +289,15 @@ namespace PR8168 {<br>
static void foo() {} // expected-error{{'static' member function \
'foo' overrides a virtual function}}<br> };<br>
}<br>
+<br>
+namespace PR26297 {<br>
+struct Incomplete;<br>
+<br>
+struct Base {<br>
+ virtual const Incomplete *meow() = 0;<br>
+};<br>
+<br>
+struct Derived : Base {<br>
+ virtual Incomplete *meow() override { return nullptr; }<br>
+};<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" \
target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br> \
</blockquote></div>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic