Replace asserts in std_code.h with invariant macros #2767
Conversation
danpoe
requested review from
kroening,
peterschrammel,
pkesseli,
smowton and
tautschnig
as code owners
src/util/std_code.h
Outdated
| @@ -143,7 +143,7 @@ class code_blockt:public codet | |||
| } | |||
| else if(statement==ID_label) | |||
| { | |||
| assert(last->operands().size()==1); | |||
| DATA_INVARIANT(last->operands().size()==1, "label has one operand"); | |||
There was a problem hiding this comment.
Phrase as a "must" or "should" statement, otherwise you get a confusing crash dump like "Invariant failed: label has one operand" which suggests the label having one operand is wrong.
danpoe
force-pushed
the
refactor/replace-asserts-std-code
branch
from
07e5cba to
1c477b7
Compare
src/util/std_code.h
Outdated
| assert(code.get_statement()==ID_dead && code.operands().size()==1); | ||
| PRECONDITION(code.get_statement() == ID_dead); | ||
| DATA_INVARIANT( | ||
| code.operands().size() == 1, "Dead code must have one operand"); |
There was a problem hiding this comment.
Better dead code -> dead statement ?
src/util/std_code.h
Outdated
| code.operands().size()==1); | ||
| PRECONDITION(code.get_statement() == ID_expression); | ||
| DATA_INVARIANT( | ||
| code.operands().size() == 1, "Expression must have one operand"); |
There was a problem hiding this comment.
If doing such cleanup-only changes, can we please get agreement and coding-standard rules on whether the error message should be lowercase or uppercase? This is not currently consistent across the code base, the coding standard mentions lowercase. I would just like to avoid that we touch those lines of code twice for cleanup-only commits.
My request-for-changes is that the rule be put in place.
|
https://github.com/diffblue/cbmc/blob/develop/CODING_STANDARD.md: "Error messages should start with a lower case letter."
Thanks for raising this issue. The rule needs to be enforced. |
|
I'll leave my request-for-changes in place for now, though with @peterschrammel's reminding me of the specific wording as a request for "please change all error messages to lowercase." |
danpoe
force-pushed
the
refactor/replace-asserts-std-code
branch
from
1c477b7 to
d1978a9
Compare
danpoe
force-pushed
the
refactor/replace-asserts-std-code
branch
from
d1978a9 to
e5032b6
Compare
|
@tautschnig I've now addressed all the comments |
No description provided.