-
Notifications
You must be signed in to change notification settings - Fork 274
move bitvector-related expression classes into separate header file #5759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
82cdacf
to
1fc4a64
Compare
1fc4a64
to
668908e
Compare
Codecov Report
@@ Coverage Diff @@
## develop #5759 +/- ##
===========================================
- Coverage 69.65% 69.65% -0.01%
===========================================
Files 1243 1245 +2
Lines 100843 100842 -1
===========================================
- Hits 70243 70242 -1
Misses 30600 30600
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going the route of splitting up std_expr.h
, then, yes, clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is a great first step to make things more manageable for humans. Now I'm wondering whether we can also do some work to help the compiler having to do a little less work, see my comment below.
src/util/bitvector_expr.h
Outdated
#ifndef CPROVER_UTIL_BITVECTOR_EXPR_H | ||
#define CPROVER_UTIL_BITVECTOR_EXPR_H | ||
|
||
/// \file util/std_expr.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy&paste error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
/// \file util/std_expr.h | ||
/// API to expression classes for bitvectors | ||
|
||
#include "std_expr.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that we need this right now, but could we consider adding something like abstract_expr.h
that declares the likes of unary_exprt
so that we can eventually speed up compile time a bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's the plan. I am considering to have unary_exprt
, binary_exprt
etc. in expr.h
.
std_expr.h is by far the largest header file, which impedes on readability. This commit moves the bitvector-related expression classes into a separate header file, following the pattern in mathematical_expr.h.
668908e
to
7b0cc28
Compare
std_expr.h
is by far the largest header file, which impedes on readability.This commit moves the bitvector-related expression classes into a separate
header file, following the pattern in
mathematical_expr.h
.