[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov 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:
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971