Quuxplusone marked 2 inline comments as done.
Quuxplusone added inline comments.


================
Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-    return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-      (!(CESK & CES_AllowRValueReferenceType) ||
-       VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+    // C++17 [class.copy.elision]p3:
----------------
mizvekov wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > A drive by fix here would be that we already have a VDType in this 
> > > context, might as well use it even though original for some reason missed 
> > > it in this part.
> > This whole block is also logically equivalent to the much simpler:
> > ```
> > if (VDType.isReferenceType()) {
> >     if (!(CESK & CES_AllowRValueReferenceType) || 
> > !VDType.isRValueReferenceType())
> >       return false;
> >     VDType = VDType.getNonReferenceType()
> > }
> > if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
> >   return false;
> > ```
> > But you do have to adjust the comments there and adjust the rest to use 
> > VDType consistently :)
> > Also, I think it might be possible to even remove the 
> > `!VDType.isObjectType() || ` part from my suggestion above, because it 
> > might be the only option left if it is not a reference anyway.
> ```
>   bool isObjectType() const {
>     // C++ [basic.types]p8:
>     //   An object type is a (possibly cv-qualified) type that is not a
>     //   function type, not a reference type, and not a void type.
>     return !isReferenceType() && !isFunctionType() && !isVoidType();
>   }
> ```
> So yeah I think you can just make my suggestion be:
> ```
> if (VDType.isReferenceType()) {
>     if (!(CESK & CES_AllowRValueReferenceType) || 
> !VDType.isRValueReferenceType())
>       return false;
>     VDType = VDType.getNonReferenceType()
> }
> if (VDType.isVolatileQualified()) 
>   return false;
> ```
> 
> 
Yeah but I //reaally// don't want to
(1) change the meaning of `VDType` in the middle of this function (mantra: "one 
name = one meaning")
(2) get "clever". I don't want to have to think about "Are there any types that 
are neither object types nor reference types?" (What about function types? What 
about block types? What about, I dunno, bit-fields?) I want the code to be 
//obviously correct//, and also to casewise match exactly what the standard 
says. It says object or rvalue reference type — let's write code that expresses 
that wording //exactly//.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98971/new/

https://reviews.llvm.org/D98971

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to