[depends: #1063] Use nullptr to represent null pointers (targets master) #1159
Conversation
|
I wholeheartedly support this PR. Yet I am wondering how we best deal with the linter warnings - are you ok dealing with them or should I pitch in and add a commit that takes care of them? |
|
I can fix them. |
|
Thanks! |
|
Some of the linter complaints refer to bigint and miniz, as these modules were modified by the nullptr pass. As these are not strictly "our" code, I'm not sure how to proceed. Should the nullptr changes be rolled-back in these files, or should I leave them in place and fix the linter errors too? |
I'm slightly in favour of rolling them back, but also any other approach is fine with me. |
|
Should the linter be adjusted to ignore "external" code like these modules? |
| @@ -22,6 +22,8 @@ Author: Daniel Kroening, [email protected] | |||
|
|
|||
| #include "interval_template.h" | |||
|
|
|||
| #define nullptr_exceptiont(str) str | |||
There was a problem hiding this comment.
I am wondering whether this is the best approach and right place to put?
There was a problem hiding this comment.
This is temporary, until #1063 gets merged. Of course this should be a custom exception type derived from invariant_failedt, and these common exception types should probably live in util. Should I add a "util/standard_exceptions.h" header and just put macros in it for now?
There was a problem hiding this comment.
Would it be ok to wait for #1063 to get merged and then do it properly?
There was a problem hiding this comment.
Yes, that's fine with me.
There was a problem hiding this comment.
Changed the PR title in a desperate attempt to introduce dependency tracking into github...
| @@ -26,6 +26,9 @@ class is_threadedt; | |||
| class dirtyt; | |||
| class reaching_definitions_analysist; | |||
|
|
|||
| #define bad_cast_exceptiont(str) str | |||
| #define nullptr_exceptiont(str) str | |||
There was a problem hiding this comment.
Now I'm convinced this isn't the right place.
tautschnig
changed the title
|
This was not ready to be merged. It required fixes which depended on #1063. |
|
Yes, I'll put together a new version once I have a chance. |
This PR was created using clang-tidy's auto-fix mode. To run clang-tidy, I had to generate a compile database, which required a CMake build. I wonder whether it would be worth creating another PR to add CMake support to CBMC master, but alongside the existing makefile build. We could add a single configuration to Travis that builds using CMake instead, to ensure that both builds are kept up-to-date.