Page MenuHomePhabricator

[analyzer][NFC] Use capital variable names, move methods out-of-line, rename some in CheckerRegistry
ClosedPublic

Authored by Szelethus on Mar 16 2019, 10:25 AM.

Details

Summary

There are barely any lines I haven't changed in these files, so I think I could might as well leave it in an LLVM coding style conforming state. I also renamed 2 functions and moved addDependency out of line to ease on followup patches.

Diff Detail

Repository
rL LLVM

Event Timeline

Szelethus created this revision.Mar 16 2019, 10:25 AM

Please rename the patch. Its name does not really express its content.

I did not check the patch yet but wanted to point out that we might not want to rush about renaming all the variables until the community decides on the coding guideline, see https://reviews.llvm.org/D59251

Szelethus retitled this revision from [analyzer][NFC] Use capital variable names in CheckerRegistry to [analyzer][NFC] Use capital variable names, move methods out-of-line, rename some in CheckerRegistry.Mar 19 2019, 2:30 AM

Please rename the patch. Its name does not really express its content.

Good point, sorry about that.

I did not check the patch yet but wanted to point out that we might not want to rush about renaming all the variables until the community decides on the coding guideline, see https://reviews.llvm.org/D59251

I wonder why this wasn't posted on cfe-dev. In any case, most of the variables are capitalized, there still is a consistency argument. Refactoring all the dependent patches would also be a pain in the buttocks for little gain.

NoQ accepted this revision.Mar 27 2019, 12:12 PM

I agree that we shouldn't try real hard to fix our code until we have a clear "party line" on how exactly do we name our variables (and ideally also how do we invalidate our caches), but having variables and fields consistent with each other makes it look much better imo, so why not.

This revision is now accepted and ready to land.Mar 27 2019, 12:12 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2019, 8:17 AM