[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness
Florin Iucha via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 10 15:57:57 PST 2020
0x8000-0000 added a comment.
In D54943#1815121 <https://reviews.llvm.org/D54943#1815121>, @JonasToth wrote:
> In D54943#1815073 <https://reviews.llvm.org/D54943#1815073>, @0x8000-0000 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 const are flagged as could be made const - I'm working to create a minimal reproduction recipe.
> > 2. We're a west const shop :) so adding a bunch of east-consts will not fly well. Is there a way to configure where 'const' is placed by the fixit? (Specifically 'auto const', we prefer it 'const auto').
>
>
> Does it build now? I couldn't find a way to reproduce that and gave up, tbh.
>
> 1. Template context? Auto involved? I saw some double-analysis for `auto&`, because clang-tidy didn't ignore those properly. And are you using `run-clang-tidy`? It deduplicates fixits, maybe that is involved?
> 2. YesNo, The utility for adding const is able to do both, west const has problems with `typedef int * MyType;` scenarios, where the `const` will apply to the wrong thing. Doing that right requires special care. BUT: `clang-format` has a east-const/west-const feature now (i think new with this release). So I am now somewhat considering to let clang-format do that for me.
>
> Thanks again for taking a look at it. But if the issues you found are new, i think we should maybe not commit this weekend.
I haven't tried Debug build, Release only.
I'm good with having clang-format do the west-const transformation.
It's not a template, but something like "const Foo foo{x, y, z};" where x and y were references and z was a pointer. I'll try to reduce a test case.
I have also noticed a check failure, but not sure if in this tool or not.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54943/new/
https://reviews.llvm.org/D54943
More information about the cfe-commits
mailing list