bitreverse_exprt: Expression to reverse the order of bits#6581
bitreverse_exprt: Expression to reverse the order of bits#6581tautschnig merged 2 commits intodiffblue:developfrom
Conversation
c1b4f99 to
f935de5
Compare
Codecov Report
@@ Coverage Diff @@
## develop #6581 +/- ##
========================================
Coverage 76.65% 76.65%
========================================
Files 1578 1579 +1
Lines 181219 181294 +75
========================================
+ Hits 138907 138973 +66
- Misses 42312 42321 +9
Continue to review full report at Codecov.
|
f935de5 to
61d9fb4
Compare
chrisr-diffblue
left a comment
There was a problem hiding this comment.
Generally looks great to me. Given the effort that went into removing unstructure exceptions in the error reporting previously, it seems a shame to be adding a throw 0 back in here - so could I kindly ask if that could be replaced with a structured exception? It'd be an approval from me if that were done :-)
src/ansi-c/c_typecheck_expr.cpp
Outdated
| error().source_location = f_op.source_location(); | ||
| error() << identifier << " expects one operand" << eom; | ||
| throw 0; |
There was a problem hiding this comment.
Throw a structured exception?
| error().source_location = f_op.source_location(); | |
| error() << identifier << " expects one operand" << eom; | |
| throw 0; | |
| std::ostringstream error_message; | |
| error_message << expr.source_location().as_string() << ": " << identifier | |
| << " expects expects one operand"; | |
| throw invalid_source_file_exceptiont{error_message.str()}; |
There was a problem hiding this comment.
Done, but I really wish we had #5837 (with its two dependencies) in place to make things even more structured!
There was a problem hiding this comment.
And now also includes a test for this one :-)
61d9fb4 to
d713c8e
Compare
chrisr-diffblue
left a comment
There was a problem hiding this comment.
Many thanks for the updates - looks great to me... And given that I poked a raw nerve on the structured exception point - feel free to assign any PRs that need reviews/comments on that front to me. 👍
d713c8e to
023fc8b
Compare
Clang has a __builtin_bitreverse (and at some point GCC might as well: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50481).
If a back-end isn't sharing aware, repeated occurrence of the operand-to-reverse could be costly.
023fc8b to
57e7b2c
Compare
Clang has a __builtin_bitreverse (and at some point GCC might as well:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50481).
Creating this as a draft: the actual implementation in the back-end is yet to be added, as are tests.