[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. In D54395#1803552 , @thakis wrote: > This breaks tests on Windows: http://45.33.8.238/win/5061/step_8.txt > > Please take a look, and if takes a while to investigate, please revert in the > meantime. (Probably needs the

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

thakis added a comment. This breaks tests on Windows: http://45.33.8.238/win/5061/step_8.txt Please take a look, and if takes a while to investigate, please revert in the meantime. (Probably needs the fno-delayed-template-instantiation kludge that's in many other tests.) Repository: rG

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf48101200ee: [clang-tidy] implement utility-function to add const to variables (authored by JonasToth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. Thank you for all the review over the time this took to land! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

aaron.ballman added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1055 + EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"), +Cat("Object const*target;")); + EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"),

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1055 + EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"), +Cat("Object const*target;")); +

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

aaron.ballman added a comment. Still LG with a small nit in one of the FIXME tests. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1006 +} + +} // namespace test JonasToth wrote: > aaron.ballman wrote: > > JonasToth wrote: > > >

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth marked 6 inline comments as done. JonasToth added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1006 +} + +} // namespace test aaron.ballman wrote: > JonasToth wrote: > > aaron.ballman wrote: > > > Can you also

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 235938. JonasToth added a comment. - add proper objc test, that fail mostly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

aaron.ballman added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:66 + StringRef S = "MyInt target = 0;"; + auto Cat = [](StringRef S) { return (T + S).str(); }; + JonasToth wrote: > aaron.ballman wrote: > > I don't think

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:66 + StringRef S = "MyInt target = 0;"; + auto Cat = [](StringRef S) { return (T + S).str(); }; + aaron.ballman

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 235838. JonasToth marked 3 inline comments as done. JonasToth added a comment. - address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this generally looks good, thank you for all the hard work on this! I just found some minor nits and testing requests. Assuming no surprises with the tests, LGTM.

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 235705. JonasToth added a comment. - clarify FIXME problem Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 235704. JonasToth added a comment. - adjust skipLParens code slightly to remove redundant checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 235703. JonasToth added a comment. - improve test situation and fix a crash coming from invalid source locations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 235611. JonasToth added a comment. - fix brace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 235610. JonasToth marked 14 inline comments as done. JonasToth added a comment. - create fresh branch with latest diff of the revision - fix review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp:34-41 +static bool isArrayType(QualType QT) { return isa(QT.getTypePtr()); } +static bool isReferenceType(QualType QT) { + return isa(QT.getTypePtr()); +} +static bool

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp:30-31 +static bool isValueType(const Type *T) { + return !(isa(T) || isa(T) || isa(T) || + isa(T)); } ObjC pointer types?

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. I know it has been a long time, but this patch is ready, i think :) i would appreciate some reviews ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 229705. JonasToth marked 2 inline comments as done. JonasToth removed a subscriber: mgehre. JonasToth added a comment. - [Misc] port patch to monorepo - add more test cases for the transformation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth marked 5 inline comments as done. JonasToth added inline comments. Herald added a subscriber: mgehre. Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bool isArrayType(QualType QT)

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

lebedev.ri added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:733 + StringRef T = "template void f(T v) \n"; + StringRef S = "{ T target = v; }"; + auto Cat = [](StringRef S) { return (T + S).str(); }; lebedev.ri wrote: > JonasToth

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

lebedev.ri added a comment. I think this looks good now, one nit about test coverage. Did you run this through your buildbot, any issues? Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:15 + +template alexfh wrote: > What's the point of default values of template arguments here? Wups, relict of older version.

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 188716. JonasToth marked 3 inline comments as done. JonasToth added a comment. - address review comments - rebase to master Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

alexfh added a comment. A few more comments. Comment at: clang-tidy/utils/FixItHintUtils.cpp:83 +return (llvm::Twine(' ') + +llvm::Twine(DeclSpec::getSpecifierName(Qualifier))) +.str(); The first operand being llvm::Twine is enough for

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. Herald added a subscriber: jdoerfert. another ping. @alexfh and @aaron.ballman you commented on prior versions. Would be nice if you could take a (final) look at this patch! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. Herald added a project: clang. ping Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. ping. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 182345. JonasToth added a comment. - generalize to DeclSpec::TQ - add dependent type-tests with `typename` Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth marked 2 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bool isArrayType(QualType QT) { return isa(QT.getTypePtr()); }

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

aaron.ballman added inline comments. Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bool isArrayType(QualType QT) { return isa(QT.getTypePtr()); } +static bool isReferenceType(QualType

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added a comment. In D54395#1309423 , @lebedev.ri wrote: > Looks reasonable to me (or at least all these tests make this look good :)), > but i'm not sure i can fully review this. No problem ;) Repository: rCTE Clang Tools Extra CHANGES

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

lebedev.ri added a comment. Looks reasonable to me (or at least all these tests make this look good :)), but i'm not sure i can fully review this. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 175444. JonasToth added a comment. reabse to master + ping :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files: clang-tidy/performance/ForRangeCopyCheck.cpp

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:733 + StringRef T = "template void f(T v) \n"; + StringRef S = "{ T target = v; }"; + auto Cat = [](StringRef S) { return (T + S).str(); }; alexfh wrote: > It would be

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth updated this revision to Diff 174828. JonasToth added a comment. add tests for different template instantiations Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54395 Files: clang-tidy/performance/ForRangeCopyCheck.cpp

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

alexfh added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:733 + StringRef T = "template void f(T v) \n"; + StringRef S = "{ T target = v; }"; + auto Cat = [](StringRef S) { return (T + S).str(); }; It would be interesting to see test

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

JonasToth created this revision. JonasToth added reviewers: aaron.ballman, hokein, alexfh, shuaiwang, lebedev.ri. Herald added subscribers: cfe-commits, xazax.hun, mgorny. JonasToth added a dependent revision: D45444: [clang-tidy] implement new check for const-correctness. This patch extends the