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

Fri May 14 07:12:51 PDT 2021

aaronpuchert added a subscriber: aaron.ballman.
aaronpuchert added a comment.

@aaron.ballman, what do you think about this? We can't really prevent anyone from writing `.getValueKind() == VK_*Value` instead of `.is*Value()`, so this change will make things consistent only for now.

In D100733#2697540 <https://reviews.llvm.org/D100733#2697540>, @mizvekov wrote:

> 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.

Not sure how to feel about this, the value categories are already hard to grasp for most C++ programmers. To solve the implicit move concerns with a new value category seems like cracking a nut with a sledgehammer. But that's not directly related to this change, we can discuss this when the change is there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100733