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

JonasToth updated this revision to Diff 424787. JonasToth added a comment. - refactor: rename check to 'misc-const-correctness' and adjust the tests accordingly - docs: adjust release notes and adjust check docs slightly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

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

JonasToth added a comment. ok. then i will continue under `misc-const-correctness`. thank you for the clarifications! 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

njames93 added a comment. I share @aaron.ballman option that given the flakiness of the guidelines, the check shouldn't mention that it addresses the c++core guidelines. Instead moving the check into another module(misc) would still provide immense value, with the side effect that it is still a

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

aaron.ballman added a comment. In D54943#3407789 , @JonasToth wrote: >> Sorry, for my own sanity, I've stopped reviewing C++ Core Guideline reviews >> for their diagnostic behavior until the guideline authors put effort into >> specifying realistic enfor

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

JonasToth added a comment. > Sorry, for my own sanity, I've stopped reviewing C++ Core Guideline reviews > for their diagnostic behavior until the guideline authors put effort into > specifying realistic enforcement guidance. And how can we continue now? I fear that this is effectively the deat

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

aaron.ballman resigned from this revision. aaron.ballman added a comment. In D54943#3394827 , @JonasToth wrote: > ping @aaron.ballman @njames93 :) Sorry, for my own sanity, I've stopped reviewing C++ Core Guideline reviews for their diagnostic behavior u

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

JonasToth added a comment. ping @aaron.ballman @njames93 :) 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.or

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

JonasToth added a comment. Another full run on LLVM with: $ ./clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \ -clang-tidy-binary ~/software/llvm-project/build_clang_tidy/bin/clang-tidy \ -checks="-*,cppcoreguidelines-const-correctness" \ -config "{CheckOptions: [{

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

JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt:33 LINK_LIBS + clangAnalysis clangTidy sammccall wrote: > JonasToth wrote: > > njames93 wrote: > > > Will

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

JonasToth updated this revision to Diff 410179. JonasToth marked 14 inline comments as done. JonasToth added a comment. - address reviews comments, part 1 - Merge branch 'main' into feature_rebase_const_transform_20210808 - fixing iterator-to-bool conversion and addressing other more review commen

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

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:102-105 + // There have been crashes on 'Variable == nullptr', even though the matcher + // is not conditional. This comes probably from 'findAll'-matching. +

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

sammccall added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt:33 LINK_LIBS + clangAnalysis clangTidy JonasToth wrote: > njames93 wrote: > > Will this work under clangd as I don't think that links in the > > cla

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

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt:33 LINK_LIBS + clangAnalysis clangTidy njames93 wrote: > Will this work under clangd as I don't think that links in the clangAnalysis > libraries @s

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

njames93 added a subscriber: sammccall. njames93 added a comment. In D54943#3296191 , @JonasToth wrote: > In D54943#3292552 , @0x8000- > wrote: > >> @aaron.ballman - can this land for Clang14, or does it have w

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

JonasToth added a comment. In D54943#3292552 , @0x8000- wrote: > @aaron.ballman - can this land for Clang14, or does it have wait for 15? > > Are there any other reviewers that can approve this? Sadly, cherry-picking to 14 is very unlikely, as this is

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

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

JonasToth 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' + // CHEC

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

LegalizeAdulthood 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' +

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

JonasToth marked 2 inline comments as done. JonasToth 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 '

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

JonasToth marked 2 inline comments as done. JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp:16 + int value_int = 42; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'value_int' of ty

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

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

LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp:16 + int value_int = 42; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'value_int' of type 'int' can be declared 'const' +}

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

JonasToth marked an inline comment as done. JonasToth added a comment. ping @aaron.ballman just in case the patch is now down on the list, I am sorry for my high latency :( Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:29 + : ClangTid

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

JonasToth updated this revision to Diff 404297. JonasToth added a comment. - use boolean for options - fix snafoo on `arc` usage Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/cl

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

JonasToth updated this revision to Diff 404296. JonasToth added a comment. - use boolean for option parsing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/clang-tidy/cppcoreguide

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

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:29 + : ClangTidyCheck(Name, Context), +AnalyzeValues(Options.get("AnalyzeValues", 1)), +AnalyzeReferences(Options.get("AnalyzeReferences", 1)),

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

JonasToth updated this revision to Diff 400296. JonasToth added a comment. - fix: adjust unit test for new array-pointer condition in range-based for loops Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Fi

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

JonasToth updated this revision to Diff 400293. JonasToth added a comment. - fix release note ordering Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/clang-tidy/cppcoreguidelines

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

JonasToth updated this revision to Diff 400291. JonasToth marked an inline comment as done. JonasToth added a comment. - improve documentation a bit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files:

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

JonasToth marked an inline comment as done. JonasToth added a comment. addressed all outstanding review comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:107-108 + int *np_local3[2] = {&np_local0[0], &np_

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

JonasToth updated this revision to Diff 400288. JonasToth marked 2 inline comments as done. JonasToth added a comment. - Merge branch 'main' into feature_rebase_const_transform_20210808 - fix: address other review comments - remove one false positive with pointers in a range-based for loop on arra

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

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31 +WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)), +TransformValues(Options.get("TransformValues", 1)), +TransformReferences

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

JonasToth updated this revision to Diff 400280. JonasToth marked 10 inline comments as done. JonasToth added a comment. - Merge branch 'main' into feature_rebase_const_transform_20210808 - fix: fixes for most review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

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

cjdb added a comment. > That said, I think this particular check is more mature and closer to landing > than the other one. Based on the amount of effort in this patch already, > unless there are strong objections, I think we should base the "make this > const if it can be made so" checks on th

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31 +WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)), +TransformValues(Options.get("TransformValues", 1)), +TransformRefere

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

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31 +WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)), +TransformValues(Options.get("TransformValues", 1)), +TransformReferences

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

aaron.ballman added a comment. In D54943#3194647 , @JonasToth wrote: > In D54943#3194643 , @aaron.ballman > wrote: > >> Thanks for pinging on this, it had fallen off my radar! @JonasToth -- did >> you ever get the

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

JonasToth added a comment. In D54943#3194643 , @aaron.ballman wrote: > Thanks for pinging on this, it had fallen off my radar! @JonasToth -- did you > ever get the chance to talk with @cjdb about the overlap mentioned in > https://reviews.llvm.org/D5494

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

aaron.ballman added a comment. Thanks for pinging on this, it had fallen off my radar! @JonasToth -- did you ever get the chance to talk with @cjdb about the overlap mentioned in https://reviews.llvm.org/D54943#2950100? (The other review also seems to have stalled out, so I'm not certain where

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

JonasToth added a comment. Herald added a subscriber: carlosgalvezp. @aaron.ballman ping for review :) I do not intend to fix the raised issues with this revision. I think it is more valuable to get this check in and get more user-feedback. But the found bugs are on the todo-list! Repository:

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

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: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

JonasToth added a comment. ping 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 https://lists.llvm.org/cgi

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

JonasToth added a comment. I further analyzer the false positive and its probably not so simple to fix. 'NoOp' is always used to add qualifiers. This is true for `const` and `__unaligned`, but there seems to be no further information to disambiguate those. Maybe this requires even something in

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

JonasToth updated this revision to Diff 370813. JonasToth added a comment. - add reproducer for '__unaligned' bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/clang-tidy/cppcor

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

JonasToth added a comment. In D54943#2956218 , @tiagoma wrote: > I am getting false positives with > > struct S{}; > > void f(__unaligned S*); > > void scope() > { > S s; > f(&s); > } This godbolt link has the AST for the examp

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

JonasToth updated this revision to Diff 370808. JonasToth added a comment. - Merge branch 'main' into feature_rebase_const_transform_20210808 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-t

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

JonasToth updated this revision to Diff 370807. JonasToth marked 2 inline comments as done. JonasToth added a comment. - minor adjustments for comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files:

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

tiagoma added a comment. In D54943#2959546 , @JonasToth wrote: > thanks for your testing! i will look at the `__unaligned` issue, not sure if > clang supports it, its an MSVC extension, is it? Yes it is. The clang frontend supports it. I am not sure abou

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

JonasToth added a comment. thanks for your testing! i will look at the `__unaligned` issue, not sure if clang supports it, its an MSVC extension, is it? Did the transformations produce issues and approximately how much code did you check? I would like to get a better feeling for its quality bef

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

tiagoma added a comment. I am getting false positives with struct S{}; void f(__unaligned S*); void scope() { S s; f(&s); } 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

tiagoma added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:168 +*Variable, *Result.Context, DeclSpec::TQ_const, +QualifierTarget::Value, QualifierPolicy::Right)) { + Diag << *Fix; -

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

I applied the patch on top of the clang13 RC and It seems not to be generating any replacements. I haven't had the time to dig and understand why. Diagnostics seem to be correct. On Sun, Aug 15, 2021 at 12:16 PM Jonas Toth via Phabricator < revi...@reviews.llvm.org> wrote: > JonasToth added a com

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

JonasToth added a comment. In D54943#2947902 , @cjdb wrote: > @JonasToth, @aaron.ballman suspects that there's enough overlap between this > patch and D107873 to consider merging the > two efforts. I'm happy to collabor

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

cjdb added a comment. @JonasToth, @aaron.ballman suspects that there's enough overlap between this patch and D107873 to consider merging the two efforts. I'm happy to collaborate, but we should probably check that our goals are aligned. Are you active on the L

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

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:145 + /// const emit multiple warnings otherwise. + const bool IsNormalVariableInTemplate = Function->isTemplateInstantiation(); + if (IsNormalVariableInTempl

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

JonasToth added a comment. The check in the latest form can properly transform `llvm/lib` and `clang/` and requires 28 manual fixes afterwards (values and references are transformed, no other modifications). Some are good and not an issue with the analysis itself, some may or may not be an iss

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

JonasToth updated this revision to Diff 366516. JonasToth added a comment. - fix: auto and type-dependent initializers fixed by ignoring them correctly in the matcher - fix docs on limitations - fix: deduplicate diagnostics from template instantiations Repository: rG LLVM Github Monorepo CHA

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

JonasToth updated this revision to Diff 366482. JonasToth added a comment. - write documentation for const-analysis check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/clang-tid

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

JonasToth updated this revision to Diff 293422. JonasToth added a comment. - remove spurious formatting change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/clang-tidy/cppcoregu

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

JonasToth updated this revision to Diff 293420. JonasToth added a comment. - rebase to revision for https://reviews.llvm.org/D88088 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra

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

JonasToth updated this revision to Diff 293418. JonasToth added a comment. - include ExprMutAnalyzer implementation changes, that got lost somehow with prior rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/

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

JonasToth added a comment. In D54943#2255358 , @AlexanderLanin wrote: > Observed behavior: > > Change: `for(string_view token : split_into_views(file_content, " \t\r\n"))` > --> `for(string_view const token : split_into_views(file_content, " > \t\r\n"))

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

JonasToth updated this revision to Diff 293392. JonasToth added a comment. - rebase to master after AST Matchers are extracted Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/CMak

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

AlexanderLanin added a comment. Observed behavior: Change: `for(string_view token : split_into_views(file_content, " \t\r\n"))` --> `for(string_view const token : split_into_views(file_content, " \t\r\n"))`. Note: `std::vector split_into_views(string_view input, const char* separators);` Proble

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

JonasToth added a comment. In D54943#1841086 , @AlexanderLanin wrote: > In D54943#1815772 , @0x8000- > wrote: > > > This generated 56 "const const" declarations, of which 25 were triple const! > This could b

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

AlexanderLanin added a comment. In D54943#1815772 , @0x8000- wrote: > This generated 56 "const const" declarations, of which 25 were triple const! I've tried this on ccache (326 fixes, nice!), but some came out as 'const const'. At least in my case

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

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

Eugene.Zelenko added a comment. In D54943#1815912 , @0x8000- wrote: > 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 */ > ... >

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

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

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

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

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

JonasToth added a comment. > Thank you for looking into this; I'll start building diff14 to test locally. Thanks :) > I'm good with merging this checker as-is, as long as it is not enabled by > default. Which part shall be disabled in your opinion? The whole check? that would not work xD But

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

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

JonasToth added a comment. @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 now (i think). If the type is deduced to a template-type or something that depends the st

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

JonasToth updated this revision to Diff 237503. JonasToth added a comment. - fix false positive with ignoring dependentTypes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/CMakeL

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

JonasToth updated this revision to Diff 237501. JonasToth added a comment. - remove pointee-analysis artifacts as this will be done later - remove unnecessary comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.or

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

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

JonasToth added a comment. In D54943#1815358 , @0x8000- wrote: > Here is a minimal example that shows the problem: > > #include > > template > struct DoGooder > { > DoGooder(void* accessor, SomeValue value) > { > } > >

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

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

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

JonasToth added a comment. In D54943#1815073 , @0x8000- wrote: > 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 con

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

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

JonasToth updated this revision to Diff 237345. JonasToth added a comment. - test if references to pointers are ignored if pointers are ignored as values - add different possible auto constellations - document the problematic cases for 'auto' deductions I think a first pass of review might make s

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

JonasToth updated this revision to Diff 237317. JonasToth added a comment. - fix unit tests for exprmutanalyzer, whitespace was the problem - Merge branch 'master' into feature_transform_const.patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5494

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

JonasToth updated this revision to Diff 237136. JonasToth added a comment. - Merge branch 'master' into feature_transform_const.patch - actually dismiss function references - work on exclusion of variables that have type, autotype or reference-type to template parameter - by default analyze the p

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

JonasToth added a comment. I did evaluate the performance of the check on LLVM with the script in the attachement. F11185973: full_transformation.sh The result, after splitting all multi-decl statements with `readability-isolate-declarations` (which produce

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

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:20 AST_MATCHER_P(LambdaExpr, hasCaptureInit, const Expr *, E) { return llvm::is_contained(Node.capture_inits(), E); https://

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

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

JonasToth updated this revision to Diff 236339. JonasToth added a comment. - fix false positive with parenListExpr, dont match so narrow for them - try removing more false positives, does not work Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/

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

JonasToth added a comment. In D54943#1805108 , @0x8000- wrote: > In D54943#1804943 , @JonasToth wrote: > > > - Merge branch 'master' into feature_transform_const.patch > > - link clangAnalysis to the cppcoreguid

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

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

JonasToth updated this revision to Diff 236259. JonasToth added a comment. - Merge branch 'master' into feature_transform_const.patch - link clangAnalysis to the cppcoreguidelines-module - fix InitListExpr as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

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

JonasToth added a comment. In D54943#1804905 , @0x8000- wrote: > In D54943#1804904 , @JonasToth wrote: > > > In D54943#1804890 , @0x8000- > > wrote: > > > > > I can'

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

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

JonasToth added a comment. In D54943#1804890 , @0x8000- wrote: > I can't build this on top of current llvm-project tree > (6a6e6f04ec2cd2f4f07ec4943036c5c2d47ce0c7 > ): > > /usr/b

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

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

JonasToth added a comment. `clang/lib` almost transforms cleanly. issues still left: - `InitListExpr` can initialize non-const references. Such cases are not figured out yet (see `create_false_positive` at the end of the testcases) - I found a case in clang, where `const` was correctly added to

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

JonasToth updated this revision to Diff 236253. JonasToth added a comment. - find more false positives - fix false positive with refernces to containers in range-for. This introduces false negative though - new test-case for check - fix range-for iterator speciality; start working on complex cond

  1   2   >