aaron.ballman added a comment.

In D132136#3753290 <https://reviews.llvm.org/D132136#3753290>, @tbaeder wrote:

> In D132136#3751702 <https://reviews.llvm.org/D132136#3751702>, @erichkeane 
> wrote:
>
>> Would be great if we had a better test here... is there anything we can do 
>> to validate this is happening other than checking for that one note?
>
> `EvaluateAsRValue` is called from `Expr::EvaluateAsRValue()`, so I think it 
> would be possible to write a unittest for this. But I think that would be a 
> lot of effort just to test this. There is even 
> `unittests/AST/EvaluateAsRValueTest.cpp` already, but it tests the wrong 
> thing :(

The existing test coverage being wrong seems like all the more reason to add 
correct test coverage. LValue to RValue conversions are important to get right 
(lol here's a wonderful demonstration of where we didn't bother to see if we 
got it right that I accidentally stumbled into when trying to give you a 
constexpr test case: https://godbolt.org/z/bdxbers3M), especially because 
they're going to impact which overload gets called when picking between an `&&` 
and `&` overload.


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

https://reviews.llvm.org/D132136

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

Reply via email to