Same as the rem patch basically.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/lib/AST/Interp/Interp.h | ||
---|---|---|
200 | Could use a helper function here and in Rem to reduce code duplication. |
clang/lib/AST/Interp/Interp.h | ||
---|---|---|
194 | Just like rem we need to catch INT_MIN / -1 CheckICE(...) looks like it does checking for both in the same spot. |
clang/test/AST/Interp/literals.cpp | ||
---|---|---|
202 | The only test coverage is for integer division; do you intend to also cover floating-point division? |
clang/test/AST/Interp/literals.cpp | ||
---|---|---|
202 | I added some float div coverage in https://reviews.llvm.org/D134859 |
clang/test/AST/Interp/literals.cpp | ||
---|---|---|
192 | Can we add a test case for INT_MIN / -1 same for % |
LGTM aside from a simple refactoring (feel free to apply it when landing or do it post-commit as an NFC change).
clang/lib/AST/Interp/Interp.h | ||
---|---|---|
203–211 | We should factor this out into a helper function so we don't have duplication between Div and Rem. Same for the above block checking for division by zero. |
I actually suggested something like this review :P I'll write it down and provide a follow-up NFC commit, thanks.
Just like rem we need to catch INT_MIN / -1
CheckICE(...) looks like it does checking for both in the same spot.