- User Since
- Jul 9 2018, 5:24 PM (144 w, 1 d)
Mar 2 2021
Add an option "IgnoredVariableNames" that allows filtering out acceptable variable names (similar to the loop counters); also implemented via regex.
Use regular expression for filtering out loop counter variables to ignore.
Addressed most of the review comments with the exception of switching to regex for ignoring loop counters; this change is in the follow-up review.
Mar 1 2021
Clean-up messy rebase for the checks/list.rst .
Rebased on main branch. Added missing documentation and release notes.
Jan 31 2021
Oct 24 2020
Would it be possible to split this review further, into "checking" and "fixing" portions? I understand that fix-it portion is more difficult, and sometimes results in multiple 'const' keywords being added, but for the past year we have used the check as part of regular CI runs to flag where it needs to be added by the engineers and for our use case it works fine that way - so I would prefer to have at least the "report" part as part of upstream Clang12, even if the 'fixup' comes later, due to the complexity of covering all corner cases.
Sep 29 2020
Sep 24 2020
Sep 23 2020
Sep 22 2020
Master branch has too many false positives for tidy - would it be possible to create a branch that contains this patch set on top of llvm-11.0-rc3? I would then add this to our internal CI.
May 27 2020
@MyDeveloperDay +1 from the trenches - I am in that same position and it took a lot of work to get the organization to _align_ on a consistent style, then enforce that.
Jan 12 2020
As an aside, once this is merged in, I dream of a "fix-it" for old style C code:
Jan 11 2020
One more mis-constification that I can't explain, nor reduce to a small test case (when I extract the code, the check behaves as expected / desired)
Summary of "misses"
I have built diff14 and tried to apply it on an internal code base with ~7500 translation units.
Jan 10 2020
Here is a minimal example that shows the problem:
Applied the fix-it on one of our smaller (but C++ code bases) and it builds and passes the tests. Comments:
Jan 6 2020
Jan 5 2020
I can't build this on top of current llvm-project tree (6a6e6f04ec2cd2f4f07ec4943036c5c2d47ce0c7):
Jan 4 2020
This patch does not build on top of current tree:
Jan 3 2020
Indicated where the new test code should go.
Dec 31 2019
Dec 22 2019
@aaron.ballman updated as suggested; please commit/integrate when you have a moment. Thank you!
Minor comment fixes; capitalization, full stop.
Dec 21 2019
My take: this change fixes a user-reported bug, and does not cause any known regressions. I think we should integrate this.
Dec 20 2019
Add a test file for expected failures
Dec 19 2019
Update release notes.
Dec 18 2019
Dec 8 2019
@aaron.ballman - can you please review this patch and if you find it acceptable please integrate it?
Dec 7 2019
Small documentation fixes.
Dec 6 2019
Flipped the default to ignore bit fields. Moved static functions out of the anonymous namespace into the clang namespace.
Dec 5 2019
Updated release notes. Removed empty lines.
Document the new 'IgnoreBitFieldsWidths' option.
Dec 4 2019
Simplified the test cases by removing extraneous code.
Dec 3 2019
Dec 2 2019
Thank you for rebasing on current master.
Sep 6 2019
Looks good to me. Thank you for the fix.
Aug 12 2018
Aug 11 2018
Rebased on curent master.
Aug 5 2018
Address several style comments. Rebase to current trunk (8.0).
Jul 31 2018
Add reference to 5.1.1 Use symbolic names instead of literal values in code in the documentation.
Add tests to show that we can detect, report and ignore floating point numbers represented using hexadecimal notation
The state of this patch:
- user interface is as agreed-upon, giving end-users the capability to filter out floats and ints as it makes sense for their code base
- code is clean
- documentation is up to date
- default ignore lists are sensible
Jul 29 2018
Update the list of magic values ignored by default.
Based on this, I think the integer list should also include 2, 3, and 4 as defaults -- those show up a lot more than I'd have expected. As for floating-point values, 1.0 certainly jumps out at me, but none of the rest seem particularly common. What do you think?
Top 40 magic numbers in https://github.com/qt/qtbase
4859 2 2901 3 1855 4 985 5 968 8 605 6 600 7 439 16 432 10 363 356 32 241 1.0f 217 12 209 255 207 100 205 9 205 20 204 50 177 0.5 174 15 162 0x2 144 24 140 0x80 135 11 127 256 113 14 110 0xff 101 1.0 99 64 99 200 96 13 86 30 84 1000 68 18 66 150 62 127 62 0xFF 58 19 58 0.05f 57 128
Add a flag to ignore all powers of two integral values.
See inline comments. Basically we need two arrays because APFloats of different semantics don't compare well, and even if we coerce them, they sometimes are not equal.
Indicate that 0 and 0.0 are accepted unconditionally (because it makes sense in the source code, and speeds-up many checks as 0s are very common and we don't want to spend log2(n) to find them at the beginning of the vector).
Jul 28 2018
Add support for ignoring specific floating point numbers.
Not trying to be difficult here - I have attempted to implement the straight-forward check.
Add option to ignore all floating point values.
I will add one more option: IgnoreFloatingPointValues, to ignore all floats and doubles, because the FloatingLiteral does not distinguish between them (as implementation detail), and I don't see a good reason to be strict about doubles and lenient about floats, or viceversa. The default value for this option is false.