[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-02-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. @aaron.ballman - can this land for Clang14, or does it have wait for 15? Are there any other reviewers that can approve this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 __

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-31 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp:12 + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double *' can be declared 'const' + // CH

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-13 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. +1 Let's get this in :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2021-10-07 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#3048763 , @JonasToth wrote: > ping Hi Jonas, Using patch32 in production (on top of "release/13.x" branch) ever since it was made available. No crashes. Only one false positive reported, which I'm not sure even is

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2021-08-13 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Using the checker now in our production BuildBot - no crashes and no false positives. Can't say if there are false negatives, but at any rate the checker is better than most colleagues at finding what should be declared const. Repository: rG LLVM Github Monorepo

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-12 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. The diff was generated with format-patch, so can be applied with "git am". The envelope: > commit d98d336b452876cfbc32ad1b76319912c45b646b (HEAD -> > readability-variable-name-length-main) > Author: Florin Iucha > Date: Mon Mar 1 00:57:01 2021 -0500 > > Add a c

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 365891. 0x8000- added a comment. One more format nit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/r

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 365879. 0x8000- added a comment. Reformatted the test file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. @aaron.ballman - thank you for the review; please submit on my behalf. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 365852. 0x8000- added a comment. Removed extra 'const' and parentheses that are not congruent with the accepted code style. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 Files: clang-tools-extra

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 365651. 0x8000- added a comment. Clean diff against origin/main CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/cl

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 365622. 0x8000- added a comment. Rebased on main and renamed from readability-variable-length to readability-identifier-length. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 Files: clang-tools-e

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:136 +CheckFactories.registerCheck( +"readability-variable-length"); } aaron.ballman wrote: > 0x8000- wrote: > > aaron.ballman wro

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-09 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:136 +CheckFactories.registerCheck( +"readability-variable-length"); } aaron.ballman wro

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-08 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 365016. 0x8000- added a comment. Updated documentation for all options. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2021-08-08 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#2933179 , @JonasToth wrote: > In D54943#2633408 , @tiagoma wrote: > >> Can we get this in? I work in Microsoft Office and we have been using this >> checker and it works great

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-07 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 364997. 0x8000- added a comment. Added separate check for parameter names. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-too

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-07 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added a comment. Added regex for exception names and rebased onto current 'main' branch. Comment at: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:136 +CheckFactories.registerCheck( +"re

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-03 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Ping? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ https://reviews.llvm.org/D97753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2021-07-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D69764#2858656 , @MyDeveloperDay wrote: > Rebase on clang12 as requested Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 _

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2021-07-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Is it possible to have this rebased on top of LLVM12 for those of us who find the trade-offs acceptable? I prefer a consistent formatting of const placement, and my code base does not have issues with the macros - if we find one that break the build, I'll suggest it

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-03-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 327655. 0x8000- added a comment. Add an option "IgnoredVariableNames" that allows filtering out acceptable variable names (similar to the loop counters); also implemented via regex. Added tests for the IgnoredVariableNames option. CHANGES SINCE LAS

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-03-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 327653. 0x8000- marked an inline comment as done. 0x8000- added a comment. Use regular expression for filtering out loop counter variables to ignore. Fully document the options. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ h

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-03-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst:9 + +Loop counter variables are expected to have a length of at least +`MinimumLoopCounterNameLength` characters

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-03-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 327649. 0x8000- added a comment. 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97753/new/ htt

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-03-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked 10 inline comments as done. 0x8000- added a comment. @Eugene.Zelenko and @njames93 Thank you both for the kind and thoughtful feedback. I am incorporating all recommended changes. First step are all the smaller scope comments, with regex support in a follow-up commit. I

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2021-01-31 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D69764#2532952 , @sammccall wrote: > In D69764#2532666 , @MyDeveloperDay > wrote: > >>> What can be done to move this change along? >> >> I feel there has to be a fundamental accepta

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-10-24 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. 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 regu

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#2292377 , @0x8000- wrote: > In D54943#2291969 , @JonasToth wrote: > >> @AlexanderLanin @0x8000- i created the branch `release-11-const` >> (https://github.com/JonasTot

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-24 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#2291969 , @JonasToth wrote: > @AlexanderLanin @0x8000- i created the branch `release-11-const` > (https://github.com/JonasToth/llvm-project/tree/release-11-const) in my fork. > > This branch is based on 11-rc3 an

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-23 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#2290713 , @AlexanderLanin wrote: >> Could you please provide me a full reproducer (optimally without >> dependencies on includes/libraries)? > > I can certainly do that based on my old version from ~9 months ago or

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-23 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#2289410 , @JonasToth wrote: > In D54943#2289037 , @0x8000- > wrote: > >> Master branch has too many false positives for tidy - would it be possible >> to create a branch

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-22 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. 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. For the legacy code (see previous reports) we found quite a few false posit

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2020-05-27 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. @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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D6976

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-12 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1815916 , @Eugene.Zelenko wrote: > In D54943#1815912 , @0x8000- > wrote: > > > As an aside, once this is merged in, I dream of a "fix-it" for old style C > > code: > > >

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-12 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. As an aside, once this is merged in, I dream of a "fix-it" for old style C code: int foo; ... /* page of code which does not use either foo or bar */ ... foo = 5; Moves the foo declaration to the line where it is actually first initialized. Then run the

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. 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) namespace util { struct StringIgnoreInitialHash : public std::unary_function { size_t operator()(cons

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Summary of "misses" One: uint32_t counter = 0; BOOST_FOREACH(const Foo* foo, *theFoos) { if (foo->getType() == someType) { ++counter; } } The -fix makes the counter const :), so obviously it breaks compilation. Two: It marks a l

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. I have built diff14 and tried to apply it on an internal code base with ~7500 translation units. $ python3 /opt/clang10/share/clang/run-clang-tidy.py -clang-tidy-binary /opt/clang10/bin/clang-tidy -clang-apply-replacements /opt/clang10/bin/clang-apply-replacemen

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1815568 , @JonasToth wrote: > @0x8000- i did remove `dependentTypes` from matching. Locally that works > with your reported test-case (i was running clang-tidy wrong :(). > > I do understand the `auto &` issues n

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1815473 , @JonasToth wrote: > In D54943#1815358 , @0x8000- > wrote: > > > Here is a minimal example that shows the problem: > > > I can not reproduce that case :( > It gi

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Here is a minimal example that shows the problem: #include template struct DoGooder { DoGooder(void* accessor, SomeValue value) { } }; struct Bingus { static constexpr auto someRandomConstant = 99; }; template s

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1815121 , @JonasToth wrote: > In D54943#1815073 , @0x8000- > wrote: > > > Applied the fix-it on one of our smaller (but C++ code bases) and it builds > > and passes the t

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Applied the fix-it on one of our smaller (but C++ code bases) and it builds and passes the tests. Comments: 1. I'm still seeing some false positives, where some locals that are const are flagged as could be made const - I'm working to create a minimal reproduction

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-06 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1805439 , @JonasToth wrote: > > This last version too builds with RelWithDebInfo and fails with Debug > > configuration (both clean builds). > > So it only fails in debug-build? Correct; I can build 'Release' or 'R

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1804943 , @JonasToth wrote: > - Merge branch 'master' into feature_transform_const.patch > - link clangAnalysis to the cppcoreguidelines-module > - fix InitListExpr as well This last version too builds with RelWithD

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1804904 , @JonasToth wrote: > In D54943#1804890 , @0x8000- > wrote: > > > I can't build this on top of current llvm-project tree > > (6a6e6f04ec2cd2f4f07ec4943036c5c2d47c

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. I can't build this on top of current llvm-project tree (6a6e6f04ec2cd2f4f07ec4943036c5c2d47ce0c7 ): /usr/bin/ld: lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in funct

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template JonasToth wrote: > JonasToth wrote: > > 0x8000- wrote:

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template JonasToth wrote: > 0x8000- wrote: > > 0x8000- wrote: > > > JonasToth wrote: > > > > JonasToth wro

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1804291 , @JonasToth wrote: > In D54943#1804183 , @0x8000- > wrote: > > > This patch does not build on top of current tree: > > > > /usr/bin/ld: > > lib/libclangTidyCpp

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. This patch does not build on top of current tree: /usr/bin/ld: lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in function `clang::tidy::cppcoreguidelines::ConstCorrectnessCheck::registerMatchers(clang::ast_matchers::MatchFinder*)': /ho

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template 0x8000- wrote: > JonasToth wrote: > > JonasToth wrote: > > > 0x8000- wrote: > > > > Please insert

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template JonasToth wrote: > JonasToth wrote: > > 0x8000- wrote: > > > Please insert the this test code here: >

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-03 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Indicated where the new test code should go. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template Please insert the this test code here: ``` struct IntWrapper {

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-03 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1803448 , @JonasToth wrote: > In D54943#1800182 , @0x8000- > wrote: > > > F11163406: 0001-Add-extra-tests.patch > > shows a couple of

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2019-12-31 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. F11163406: 0001-Add-extra-tests.patch shows a couple of false positives. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943

[PATCH] D71686: Fix false positive in magic number checker

2019-12-22 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. @aaron.ballman updated as suggested; please commit/integrate when you have a moment. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71686/new/ https://reviews.llvm.org/D71686 ___ cfe-commits mailing l

[PATCH] D71686: Fix false positive in magic number checker

2019-12-22 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked 2 inline comments as done. 0x8000- added a comment. In D71686#1794366 , @aaron.ballman wrote: > In D71686#1794360 , @0x8000- > wrote: > > > In D71686#1794330

[PATCH] D71686: Fix false positive in magic number checker

2019-12-22 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 235060. 0x8000- added a comment. Minor comment fixes; capitalization, full stop. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71686/new/ https://reviews.llvm.org/D71686 Files: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cp

[PATCH] D71686: Fix false positive in magic number checker

2019-12-22 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D71686#1794330 , @aaron.ballman wrote: > In D71686#1794053 , @0x8000- > wrote: > > > My take: this change fixes a user-reported bug, and does not cause any > > known regression

[PATCH] D71686: Fix false positive in magic number checker

2019-12-21 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. My take: this change fixes a user-reported bug, and does not cause any known regressions. I think we should integrate this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71686/new/ https://reviews.llvm.org/D71686 _

[PATCH] D71686: Fix false positive in magic number checker

2019-12-20 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers-todo.cpp:9 +{ + if (((int)4) > ProcessSomething(10)) + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 4 is a magic nu

[PATCH] D71686: Fix false positive in magic number checker

2019-12-20 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 234998. 0x8000- added a comment. Add a test file for expected failures CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71686/new/ https://reviews.llvm.org/D71686 Files: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp clang-

[PATCH] D71686: Fix false positive in magic number checker

2019-12-20 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp:129 +// expanded class enumeration value. +if (Parent.get()) +

[PATCH] D71686: Fix false positive in magic number checker

2019-12-20 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp:129 +// expanded class enumeration value. +if (Parent.get()) +

[PATCH] D71686: Fix false positive in magic number checker

2019-12-19 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 234819. 0x8000- added a comment. Fix typo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71686/new/ https://reviews.llvm.org/D71686 Files: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp clang-tools-extra/docs/ReleaseNote

[PATCH] D71686: Fix false positive in magic number checker

2019-12-19 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 234818. 0x8000- edited the summary of this revision. 0x8000- added a comment. Update release notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71686/new/ https://reviews.llvm.org/D71686 Files: clang-tools-extra/clang-tidy/readabili

[PATCH] D71686: Fix false positive in magic number checker

2019-12-18 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp:127 + +// Ignore this instance, because this matches an +// expanded class e

[PATCH] D71686: Fix false positive in magic number checker

2019-12-18 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- created this revision. 0x8000- added a reviewer: aaron.ballman. 0x8000- added a project: clang-tools-extra. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix false positive in magic number checker https://bugs.llvm.org/show_bug.cgi?id=40640: cppcoregu

[PATCH] D67265: [clang-tidy] Magic number checker shouldn't warn on user defined string literals

2019-12-08 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Herald added a subscriber: mgehre. @aaron.ballman - can you please review this patch and if you find it acceptable please integrate it? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67265/new/ https://reviews.llvm.org/

[PATCH] D71043: Exclude bitfield definitions from magic numbers check

2019-12-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- created this revision. 0x8000- added reviewers: JonasToth, lebedev.ri, aaron.ballman, alexfh. Herald added a project: clang. Herald added a subscriber: cfe-commits. 0x8000- removed a reviewer: lebedev.ri. 0x8000- edited projects, added clang-tools-extra; removed clang. 0x800

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-12-03 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D45444#1766852 , @JonasToth wrote: > > Thank you for testing, I would appreciate if you test later versions, too! > I will focus on landing the utility function for adding `const` first. I > noticed false positives in

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-12-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Thank you for rebasing on current master. I have ran it today on our code base and found three classes of false positives: 1. Writing to a bitfield of a struct. The struct still is suggested it should be const. 2. Using a variable with an ostream extraction; like "i

[PATCH] D67265: [clang-tidy] Magic number checker shouldn't warn on user defined string literals

2019-09-06 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- accepted this revision. 0x8000- added a comment. This revision is now accepted and ready to land. Looks good to me. Thank you for the fix. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67265/new/ https://reviews.llvm.org/D67265 __

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-08-12 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In https://reviews.llvm.org/D49114#1196480, @aaron.ballman wrote: > Alex has had plenty of time to respond, so I'm fine handling any concerns he > has post-commit. Do you need me to commit this on your behalf? Yes, please! Thank you! Author: "Florin Iucha " Repo

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-08-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 160243. 0x8000- added a comment. Rebased on curent master. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability/CMakeLists.txt clang-ti

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-08-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 159222. 0x8000- added a comment. Address several style comments. Rebase to current trunk (8.0). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-31 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 158450. 0x8000- added a comment. Add reference to 5.1.1 Use symbolic names instead of literal values in code in the documentation. Repository: rCTE C

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-31 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 158425. 0x8000- added a comment. Add tests to show that we can detect, report and ignore floating point numbers represented using hexadecimal notation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcore

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-31 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. 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 There is one outstandin

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 157899. 0x8000- added a comment. Update the list of magic values ignored by default. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. > 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 th

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. 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

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 157889. 0x8000- added a comment. Add a flag to ignore all powers of two integral values. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readabi

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:57 +const char DefaultIgnoredIntegerValues[] = "0;1;"; +const char DefaultIgnoredFloatingPointValues[] = "0.0;"; + aaron.ballman wrote: > 0x8000- wrote: > > aaron.ball

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:57 +const char DefaultIgnoredIntegerValues[] = "0;1;"; +const char DefaultIgnoredFloatingPointValues[] = "0.0;"; + aaron.ballman wrote: > 0x8000- wrote: > > aaron.ball

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked 2 inline comments as done. 0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked 4 inline comments as done. 0x8000- added a comment. 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. Comment at: clang-tidy/readabilit

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 157886. 0x8000- added a comment. 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

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 157879. 0x8000- added a comment. Add support for ignoring specific floating point numbers. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/reada

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Not trying to be difficult here - I have attempted to implement the straight-forward check. Added this to the MagicNumbersCheck::MagicNumbersCheck constructor: + // process set of ignored floating point values + const std::vector IgnoredFloatingPointValuesInpu

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. ---

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. ---

  1   2   >