[PATCH] D131888: [Clang][Interp] Add some basic functionality to the experimental new constant interpreter

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

So you're right, we haven't seen much work on this interpreter in a while, and I have no real experience with reviewing it at all, which I'm sure is the experience of most of my co-reviewers.

At the same time, I believe there is good potential in this interpreter, so I would love to see progress on it.

SO, if you could start by splitting this patch up into the smallest parts possible, it would be really appreciated.  This patch seems to be doing a lot, and I'm a bit overwhelmed trying to review it all at once.

Additionally, I suspect there is going to be a LOT of tests, so I'd suggest writing 1 whole 'test file' per topic (instead of throwing everything into interp.cpp).  I also suspect that there is need for more in depth testing than that.  For exmaple, 'if' statements likely need to check things like side-effects and init statements/etc.

One valuable strategy might be to temporarily switch this over to the 'default' interpreter in your local workspace, make changes, and see if you can add a new 'run' line to any existing tests that each individual change you make causes to pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131888