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