rsmith added a comment.

I think this is worth doing -- "rvalue" is at least ambiguous and, in 
C++-specific cases, confusing and wrong. Saying "prvalue" in C-specific parts 
of clang may also be a bit surprising, but it's unambiguous and still 
meaningful.

I think we shouldn't consider adding an `isRValue` until the dust has settled 
and we're used to using "prvalue" for prvalues. Even then, we should be 
cautious about it, because people might reach for it when they mean prvalue 
(and in any case I'd expect `isRValue()` checks to be quite rare.



================
Comment at: clang/include/clang/AST/Expr.h:266-271
   /// C++11 divides the concept of "r-value" into pure r-values
   /// ("pr-values") and so-called expiring values ("x-values"), which
   /// identify specific objects that can be safely cannibalized for
   /// their resources.  This is an unfortunate abuse of terminology on
   /// the part of the C++ committee.  In Clang, when we say "r-value",
   /// we generally mean a pr-value.
----------------
This comment needs an update.


================
Comment at: clang/lib/AST/ExprConstant.cpp:2508
   assert(!E->isValueDependent());
-  assert(E->isRValue() && "missing lvalue-to-rvalue conv in bool condition");
+  assert(E->isPRValue() && "missing lvalue-to-prvalue conv in bool condition");
   APValue Val;
----------------
I'd leave the assert message alone here: for (better or) worse, C++ calls this 
conversion an "lvalue-to-rvalue conversion" even though it converts glvalues to 
prvalues.


================
Comment at: clang/lib/Sema/Sema.cpp:583
+      llvm_unreachable(
+          ("can't implicitly cast lvalue to prvalue with this cast "
+           "kind: " +
----------------



================
Comment at: clang/lib/Sema/Sema.cpp:597
+  assert((VK == VK_PRValue || Kind == CK_Dependent || !E->isPRValue()) &&
+         "can't cast prvalue to lvalue");
 #endif
----------------



================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5662
   switch (ET) {
   case ET_IsLValueExpr: return E->isLValue();
+  case ET_IsRValueExpr:
----------------
Hm, I wonder if this it's correct that these both evaluate to false for an 
xvalue. Does anyone have a copy of Embarcadero's 32-bit compiler to test with?


================
Comment at: clang/lib/Sema/SemaInit.cpp:3588
+  case VK_PRValue:
+    S.Kind = SK_CastDerivedToBaseRValue;
+    break;
----------------
Would be nice to rename this `SK_` enumerator as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103720

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

Reply via email to