-
Notifications
You must be signed in to change notification settings - Fork 43
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
Linting the code #218
Comments
Generally the cleanups are great and useful, but if they touch a lot of code, it can make history tracking (git blame) harder. How large is the diff? |
These are isolated single-line changes, so blame would change for only that line:
For reference, there are 1309 *.cc files and 124 *.icc files in the project. |
OK, please go ahead then. |
Will do. Please apply #219 first (UTF-8 fixes) as I cannot run the formatter on these new changes until that's fixed. |
Since the formatting script is having a bit of trouble (it sees a non-UTF8 character in the "before" line and crashes), I'm having to break up the changes and isolate the problem. The first change -- replacing system headers with c++ headers -- is in #221 . The formatting script may have done more than you want in openasip/opset/base/base.cc and openasip/opset/base/double.cc, so let me know if I need to make adjustments (either changing the line length value from 78 or marking these as noformat). |
I've been running a linter (clang-tidy) over the code for the past month and testing the the results of the recommended fixes. Are you interested in any or all of these? (I have all of these working in a private branch.)
<math.h>
to<cmath>
. The compiler uses its own bundled headers, so supplying a system header can lead to subtle bugs if there are layout differences between the two.Updates to use C++11 syntax and features:
nullptr
to replace NULL and 0 as appropriate. This makes the use of null pointers more explicit.auto
orauto*
for when initializing objects where the type is obvious to avoid redundant declarations (Foo* foo = new Foo()
toauto* foo = new Foo()
override
to label method overrides (instead of usingvirtual
to mean both "can override" and "is overridden".)virtual override
means you're overriding and someone else can override that. This also silences numerous compiler warnings in Clang.typedef
withusing
as appropriate. This is syntactic sugar that makes code easier to read.typedef std::map<BitVector, unsigned int> Dictionary;
becomesusing Dictionary = std::map<BitVector, unsigned int>;
=default
to have the compiler generate the appropriate code for ctors, dtors, copy/move, etc. when you have to override one of those but leave the rest alone. Example:Old code:
The text was updated successfully, but these errors were encountered: