Page MenuHomePhabricator

[analyzer] Run clang-format and fix style
ClosedPublic

Authored by ddcc on Nov 15 2016, 12:45 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

ddcc updated this revision to Diff 78054.Nov 15 2016, 12:45 PM
ddcc retitled this revision from to [analyzer] Run clang-format and fix style.
ddcc updated this object.
ddcc added reviewers: zaks.anna, dcoughlin.
ddcc added a subscriber: cfe-commits.
zaks.anna edited edge metadata.Nov 30 2016, 10:56 PM
zaks.anna added a subscriber: dergachev.a.

I am not a big fan of loosing svn blame only to fix formatting, but since you are modifying this code anyway, it's fine by me.

Artem and Devin, what is your opinion on this?

lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
459 ↗(On Diff #78054)

We should use lower case function names.

lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
228 ↗(On Diff #78054)

I don't think we should change the case in the comment.

255 ↗(On Diff #78054)

Not sure the comment needs to change.

dcoughlin edited edge metadata.Dec 1 2016, 11:19 AM

I am not a big fan of loosing svn blame only to fix formatting, but since you are modifying this code anyway, it's fine by me.

Artem and Devin, what is your opinion on this?

I agree that in general it is not good to lose the annotation information, but if you're fine with it I'm fine with it.

ddcc updated this revision to Diff 80128.Dec 2 2016, 1:48 PM
ddcc edited edge metadata.

Un-change comments, misc. changes

lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
459 ↗(On Diff #78054)

Are you saying more functions should be changed to lowercase (e.g. intersect)? Or that getRange should be getrange?

zaks.anna added inline comments.Dec 2 2016, 6:16 PM
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
459 ↗(On Diff #78054)

Should be "camel case, and start with a lower case letter", see:

http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

ddcc added inline comments.Dec 2 2016, 8:14 PM
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
459 ↗(On Diff #78054)

Right, isn't changing GetRange to getRange correct then? I'm a little confused, is there something else that needs to be fixed?

zaks.anna accepted this revision.Dec 9 2016, 11:21 PM
zaks.anna edited edge metadata.

Thank you!

lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
459 ↗(On Diff #78054)

Sorry, my bad. This does look good!

This revision is now accepted and ready to land.Dec 9 2016, 11:21 PM
This revision was automatically updated to reflect the committed changes.