[PATCH] D103720: [clang] NFC: Rename rvalue to prvalue

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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