tbaeder added a comment.
In D132136#3766958 <https://reviews.llvm.org/D132136#3766958>, @aaron.ballman
wrote:
> On the one hand, I'm not comfortable landing this without adequate testing,
> especially because you want to build more stuff on top of it. I think we need
> to make sure the foundational bits of evaluation are rock solid before
> building things on top of them. However, on the other hand, this is
> foundational enough that it's hard to test this bit without other parts of
> the interpreter being ready. So my preference is to try to add test coverage
> even if it's failing currently because other parts of the interpreter aren't
> ready yet. e.g., (imaginary test case, feel free to use better ones)
>
> template <typename Ty, typename Uy>
> struct is_same { static constexpr bool value = false; };
>
> template <typename Ty>
> struct is_same<Ty, Ty> { static constexpr bool value = true; };
>
> const volatile int i = 0;
> // Test that lvalue to rvalue conversion ends up stripping top-level
> qualifiers.
> // FIXME: this isn't correct yet because we don't have support for
> qualifier conversions yet.
> static_assert(is_same<decltype(+i), int>::value, "oh no!"); //
> expected-error {{static assertion failed: oh no!}}
>
> This way, we can start getting extensive test cases written while thinking
> about the situations specific to lvalue to rvalue conversion (or whatever
> else is being implemented at the moment) instead of trying to come back and
> remember to write them later.
>
> Is that reasonable, or is the interpreter in such a state where even this
> kind of testing is basically impossible to come up with?
Soo... I was gonna tell you that the test case is obviously not going to work
right now, since record types aren't implemented at all. //But// the test case
actually works just fine. However, it also works without the lvalue-to-rvalue
conversion in `EvaluteAsRValue`. Both with the old and the new interpreter.
I'm going to add an actual unit test that fails without the conversion. But it
only checks a very simple case, i.e. that a `DeclRefExpr` is returned as an
integer and not the lvalue.
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
