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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
