-
Notifications
You must be signed in to change notification settings - Fork 193
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
Misc Refactoring/Improvements #387
Conversation
… to improve runtime for hmmer test by 0.06ms.
This is going to fail the style tests for several files. clang-format changed behaviour between 3.8 and 3.9 (I think you could call it a bugfix) It looks like you're using clang-format-4.0 or so, but the check uses 3.8. I suggest you remove all the files where you are only changing style. They can be updated when the style checker is updated. That will be done when it looks like clang-format-3.9+ is more common. |
Ahh, ok. Thank you, that probably just saved me an evening of torture. I'll fix it up and push the changes. |
src/PrecedenceGraph.h
Outdated
|
||
public: | ||
static constexpr const char* name = "scc-graph"; | ||
|
||
void run(const AstTranslationUnit& translationUnit) override; | ||
|
||
int getSCCForRelation(const AstRelation* relation) { | ||
return nodeToSCC[relation]; | ||
const unsigned getSCCForRelation(const AstRelation* relation) const { |
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.
Do these non-reference returns really need to return a const?
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.
No there is no reason for them to do that and I shall change it back.
This pull request bundles a bunch of very small changes, each too minor to justify its own pull request but taken as a whole too major to simply bundle with another.
They include: