[PATCH] D100733: [clang] NFC: change uses of `Expr->getValueKind` into `is?Value`

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D100733#2697537 <https://reviews.llvm.org/D100733#2697537>, @aaronpuchert wrote:

> The change seems to be correct, but I'm wondering if `x.getValueKind() == VK_*Value` doesn't have one advantage over `x.is*Value()`: it's obvious that this is exclusive with the other values. Especially with `isRValue()` it might not be so obvious, because Clang doesn't follow the C++11 terminology with this.
>
> But it's admittedly shorter, so I'd be willing to approve this.

This came up in a patch where I am experimenting with a new value category.
The helpers 'help' a lot more when you are changing some of these tests from testing just one category, to testing a combination of categories (like GLValue).

But this is just the easy pickings of the patch, which is still WIP, and I may need in the future for some more drastic change to deal with the code that just stores the kind in a variable and tests that later.

It would be useful if you could do:

  VK = Expr->getValueKind();
  if (VK.isGLValue()) {

I may need to do something like that later.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:5522
     }
     VK = LHSExp->getValueKind();
     if (VK != VK_RValue)
----------------
aaronpuchert wrote:
> There might be a certain benefit to using `LHSExp->getValueKind()` above when we use it here again: that makes it more obvious what we're trying to achieve in that `if`. (Namely changing the value category.)
Or just making the same kind of change here again:
```
if (!LHSExp->isRValue())
  OK = OK_VectorComponent;
VK = LHSExp->getValueKind();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100733