Page MenuHomePhabricator

JonasToth (Jonas Toth)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 31 2016, 11:13 AM (241 w, 3 d)

Recent Activity

Nov 13 2020

JonasToth accepted D90972: [clang-tidy] Install run-clang-tidy.py in bin/ as run-clang-tidy.

LGTM! thanks for fixing.

Nov 13 2020, 10:06 AM · Restricted Project, Restricted Project

Oct 12 2020

JonasToth added reviewers for D75813: [clang-tidy] fix readability-braces-around-statements Stmt type dependency: njames93, JonasToth.
Oct 12 2020, 4:13 AM · Restricted Project, Restricted Project
JonasToth added a comment to D75813: [clang-tidy] fix readability-braces-around-statements Stmt type dependency.

Hey Alex, first thanks for the bug fixing! Of course we are interested in improvements in clang-tidy, but i think noone is actually employed to help out here and the normal reviewers have many, many reviews parallel. Sometimes stuff just slips through :/

Oct 12 2020, 4:13 AM · Restricted Project, Restricted Project

Oct 11 2020

JonasToth added a comment to D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs.

You can land it. Worst case would be post-commit comments, but this is not a complicated patch and should be fine!

Oct 11 2020, 8:44 AM · Restricted Project, Restricted Project
JonasToth accepted D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs.

I am fine now, thank you!

Oct 11 2020, 8:31 AM · Restricted Project, Restricted Project
JonasToth added a comment to D79113: Revert "Remove false positive in AvoidNonConstGlobalVariables.".

The issue is resolved now in the CppCoreGuidelines.

Oct 11 2020, 8:22 AM · Restricted Project
JonasToth added a comment to D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs.

The context thingie was not that important here, but still thanks :)

Oct 11 2020, 8:16 AM · Restricted Project, Restricted Project
JonasToth added inline comments to D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs.
Oct 11 2020, 8:02 AM · Restricted Project, Restricted Project
JonasToth accepted D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs.

LGTM.
Short reminder to please use full context patches. Its not an issue right now, but review sometimes just needs the context to understand the changes :)

Oct 11 2020, 8:01 AM · Restricted Project, Restricted Project

Oct 10 2020

JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • fix test RUN line
Oct 10 2020, 5:43 AM · Restricted Project, Restricted Project
JonasToth added inline comments to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
Oct 10 2020, 5:12 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • no delayed template parsing
  • adjust release note issue
Oct 10 2020, 5:11 AM · Restricted Project, Restricted Project

Oct 9 2020

JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • missed one place of decltype
Oct 9 2020, 7:23 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • fix platform dependent warning message for decltype to just match the beginning and not the 'a.k.a'
Oct 9 2020, 6:43 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • address review comments
Oct 9 2020, 6:04 AM · Restricted Project, Restricted Project
JonasToth added inline comments to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
Oct 9 2020, 6:02 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • update to landed ExprMutAnalyzer changes
Oct 9 2020, 5:09 AM · Restricted Project, Restricted Project
JonasToth committed rGe517e5cfec90: [clang] improve accuracy of ExprMutAnalyzer (authored by JonasToth).
[clang] improve accuracy of ExprMutAnalyzer
Oct 9 2020, 4:46 AM
JonasToth closed D88088: [clang] improve accuracy of ExprMutAnalyzer.
Oct 9 2020, 4:46 AM · Restricted Project
JonasToth added inline comments to D88088: [clang] improve accuracy of ExprMutAnalyzer.
Oct 9 2020, 4:16 AM · Restricted Project
JonasToth updated the diff for D88088: [clang] improve accuracy of ExprMutAnalyzer.
  • [Feature] add elvis operator detection
Oct 9 2020, 4:15 AM · Restricted Project

Oct 7 2020

JonasToth updated the diff for D88088: [clang] improve accuracy of ExprMutAnalyzer.
  • fix imprecisions
Oct 7 2020, 2:41 AM · Restricted Project

Oct 6 2020

JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

@AlexanderLanin @0x8000-0000 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 and cherry picks the commits that correspond to this revision.
I hope this is of any use for you! I will probably forget to update this branch all the time, but if I feel like there has been actual progress made, i will update!

Thank you Jonas - will add this to our CI tool for the new project and try to find some time to run it on the legacy project.

Just a quick update; in clang-tidy mode it works great, with no false positive and no crashes. I haven't tried the 'apply-replacements' mode.

Oct 6 2020, 2:53 PM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

rebase to newest expmutanalyzer patch

Oct 6 2020, 2:35 PM · Restricted Project, Restricted Project
JonasToth retitled D88088: [clang] improve accuracy of ExprMutAnalyzer from WIP [clang] improve accuracy of ExprMutAnalyzer to [clang] improve accuracy of ExprMutAnalyzer.
Oct 6 2020, 2:32 PM · Restricted Project
JonasToth added a comment to D88088: [clang] improve accuracy of ExprMutAnalyzer.

@aaron.ballman I update the code a bit. This stuff is just a hydra :/

Oct 6 2020, 2:32 PM · Restricted Project
JonasToth updated the diff for D88088: [clang] improve accuracy of ExprMutAnalyzer.
  • document sections in the testcases, hopefully little more structure for comprehension
  • fix derived-to-base-cast OUTSIDE of an conditional operator
  • improve type-dependent callexpr, too
  • better canResolveToExpr matcher code
  • adjust matcher for comma
Oct 6 2020, 2:32 PM · Restricted Project
JonasToth added a comment to D54222: [clang-tidy] Add a check to detect returning static locals in public headers.

I am confused now, but richard knows this stuff very well! The patches you referenced showed only cases where the static was defined in an inline-header definition (and templates). Does this make a difference on the semantics?
This should be clear before we continue.

Oct 6 2020, 1:37 AM · Restricted Project, Restricted Project

Oct 5 2020

JonasToth retitled D88833: [clang-tidy] Do not warn on pointer decays in system macros from Do not warn on pointer decays in system macros to [clang-tidy] Do not warn on pointer decays in system macros.
Oct 5 2020, 10:51 AM · Restricted Project, Restricted Project

Sep 26 2020

JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

Off-Topic: I was just attempting to apply this to my codebase at work.
Seems this will be more challenging than anticipated 😉
My best guess is this is related to D72730

Applying fixes ...
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
Sep 26 2020, 8:57 AM · Restricted Project, Restricted Project

Sep 25 2020

JonasToth added a comment to D54222: [clang-tidy] Add a check to detect returning static locals in public headers.

I agree with @Eugene.Zelenko that this check should rather live in bugprone- as the issue its diagnosing is not LLVM specific. It is ok to add an alias into the llvm module.

Here are the warnings it issues with the patch's matcher:

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:625:43: warning: address of static local variable 'tag' may not be identical across library boundaries [llvm-problematic-statics]
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:219:1: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h:25:1: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]

And here are the additional warnings it issues when relaxing the matcher (to be unless(isExpansionInMainFile())):

clang/lib/StaticAnalyzer/Checkers/Iterator.h:130:47: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/lib/StaticAnalyzer/Checkers/Iterator.h:136:47: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/lib/StaticAnalyzer/Checkers/Iterator.h:142:47: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h:60:5: warning: address of static local variable 'index' may not be identical across library boundaries [llvm-problematic-statics]
llvm/utils/unittest/googletest/src/gtest.cc:3989:3: warning: address of static local variable 'instance' may not be identical across library boundaries [llvm-problematic-statics]

And also this one, which was hidden by default but made running tidy noisier by always saying there was a hidden warning:

/usr/bin/../include/c++/v1/functional:1960:9: warning: address of static local variable '__policy_' may not be identical across library boundaries [llvm-problematic-statics]

All of the first set of warnings might be actual problems, but the second set definitely aren't problems because they aren't going to be used across shared library boundaries. By making this be a LLVM-specific check, we can avoid the noise of these false positives. Also, I looked through the C++ Core Guidelines for any rules that might address this but didn't see anything relevant.

Thoughts?

Sep 25 2020, 10:14 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D88088: [clang] improve accuracy of ExprMutAnalyzer.
  • fix typo that provided wrong argument to AST building
Sep 25 2020, 7:21 AM · Restricted Project
JonasToth updated the diff for D88088: [clang] improve accuracy of ExprMutAnalyzer.
  • address review comments
Sep 25 2020, 7:19 AM · Restricted Project

Sep 24 2020

JonasToth added inline comments to D88088: [clang] improve accuracy of ExprMutAnalyzer.
Sep 24 2020, 7:24 AM · Restricted Project
JonasToth updated the diff for D88088: [clang] improve accuracy of ExprMutAnalyzer.
  • adress review comments
Sep 24 2020, 7:24 AM · Restricted Project
JonasToth committed rG4e534900476d: [NFC][Docs] fix clang-docs compilation (authored by JonasToth).
[NFC][Docs] fix clang-docs compilation
Sep 24 2020, 4:14 AM
JonasToth added a comment to D36836: [clang-tidy] Implement readability-function-cognitive-complexity check.

Rebased.

There is a number of official open-source LGPL-3 implementations already:

There are other open-source LGPL-3 implementations already:

There are other 3rd party implementations:

Quite honestly, i do not understand how did the license question arose.
Would have it been fine if i based this on the open-source-licensed code?
Would have it not been? Would same license question be raised?
Somehow i don't think it would have been.

Is this really just about Copyright SonarSource S.A., 2018, Switzerland. All content is copyright protected. in https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
But that is only about the document, not the algorithm.
But even if we enternain the idea that all of the implementations must bow to that license,
then surely this is not the first code in LLVM that is implicitly/explicitly based on copyrighted doc.

This is rather frustrating.

Sep 24 2020, 2:04 AM · Restricted Project, Restricted Project
JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

@AlexanderLanin @0x8000-0000 i created the branch release-11-const (https://github.com/JonasToth/llvm-project/tree/release-11-const) in my fork.

Sep 24 2020, 1:54 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
rebase to current exprmutanalyzer
Sep 24 2020, 1:36 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D88088: [clang] improve accuracy of ExprMutAnalyzer.
  • improve the expression matcher, to catch the expression in general ternary-operator cases, too
  • considered unresolved operator calls to be a modification
  • fix method calls with templates in some instances
Sep 24 2020, 1:30 AM · Restricted Project

Sep 23 2020

JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

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.

Sep 23 2020, 12:02 AM · Restricted Project, Restricted Project

Sep 22 2020

JonasToth added a comment to D87588: [ASTMatchers] extract public matchers from const-analysis into own patch.
In D87588#2278948, @ro wrote:

As described in D87825, this patch broke Debug builds on sparcv9-sun-solaris2.11 and amd64-pc-solaris2.11.

Sep 22 2020, 9:44 AM · Restricted Project
JonasToth added a comment to D54222: [clang-tidy] Add a check to detect returning static locals in public headers.

@JonasToth It looks like there are some outstanding review comments that need to be addressed. I've gotten some time allocated to work on this next week.

Sep 22 2020, 9:41 AM · Restricted Project, Restricted Project
JonasToth retitled D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness from [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness to WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
Sep 22 2020, 8:07 AM · Restricted Project, Restricted Project
JonasToth retitled D88088: [clang] improve accuracy of ExprMutAnalyzer from [clang] improve accuracy of ExprMutAnalyzer to WIP [clang] improve accuracy of ExprMutAnalyzer.
Sep 22 2020, 8:06 AM · Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • remove spurious formatting change
Sep 22 2020, 5:21 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
Sep 22 2020, 5:17 AM · Restricted Project, Restricted Project
JonasToth requested review of D88088: [clang] improve accuracy of ExprMutAnalyzer.
Sep 22 2020, 5:16 AM · Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • include ExprMutAnalyzer implementation changes, that got lost somehow with prior rebase
Sep 22 2020, 5:12 AM · Restricted Project, Restricted Project
JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

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<string_view> split_into_views(string_view input, const char* separators);
Problem: Now it triggers error: loop variable 'token' of type 'const nonstd::sv_lite::string_view' (aka 'const basic_string_view<char>') creates a copy from type 'const nonstd::sv_lite::string_view' [-Werror,-Wrange-loop-analysis]
Fixed manually by making it for(std::string_view& const token : split_into_views(file_content, " \t\r\n")).

Sep 22 2020, 3:02 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • rebase to master after AST Matchers are extracted
Sep 22 2020, 2:59 AM · Restricted Project, Restricted Project

Sep 15 2020

JonasToth committed rG69f98311ca42: [ASTMatchers] extract public matchers from const-analysis into own patch (authored by JonasToth).
[ASTMatchers] extract public matchers from const-analysis into own patch
Sep 15 2020, 12:08 PM
JonasToth closed D87588: [ASTMatchers] extract public matchers from const-analysis into own patch.
Sep 15 2020, 12:08 PM · Restricted Project
JonasToth updated the diff for D87588: [ASTMatchers] extract public matchers from const-analysis into own patch.
  • rebase
Sep 15 2020, 12:06 PM · Restricted Project

Sep 14 2020

JonasToth updated the diff for D87588: [ASTMatchers] extract public matchers from const-analysis into own patch.
  • fix compilation error from removed brace
Sep 14 2020, 9:14 AM · Restricted Project

Sep 13 2020

JonasToth updated the diff for D87588: [ASTMatchers] extract public matchers from const-analysis into own patch.
  • fix left over merge marker and one formatting problem
Sep 13 2020, 10:44 AM · Restricted Project
JonasToth requested review of D87588: [ASTMatchers] extract public matchers from const-analysis into own patch.
Sep 13 2020, 10:39 AM · Restricted Project
JonasToth accepted D72553: [clang-tidy] Add performance-prefer-preincrement check.

Sorry for missing this review, i simply hadn't time :/
All my concerns are addressed and I think this is LGTM after rebasing.

Sep 13 2020, 3:48 AM · Restricted Project, Restricted Project
JonasToth added a comment to D54222: [clang-tidy] Add a check to detect returning static locals in public headers.

ping.
This checks seems pretty much done. @jranieri-grammatech do you think there is something missing? Otherwise it could be rebased and comitted?

Sep 13 2020, 3:20 AM · Restricted Project, Restricted Project
JonasToth resigned from D72730: [clang-tidy] run-clang-tidy -export-fixes exports only fixes, not all warnings.
Sep 13 2020, 3:07 AM · Restricted Project, Restricted Project
JonasToth resigned from D74813: Function block naming - add hash of parameter type to end of block name.
Sep 13 2020, 2:56 AM · Restricted Project, Restricted Project
JonasToth added a comment to D75107: [clang-tidy] hicpp-signed-bitwise IgnorePositiveIntegerLiterals now partially recursive.

ping.
@njames93 are you revisiting this patch? What is the missing piece? Maybe i can help out a bit?

Sep 13 2020, 2:55 AM · Restricted Project

Sep 10 2020

JonasToth accepted D82784: [clang-tidy] For `run-clang-tidy.py` do not treat `allow_enabling_alpha_checkers` as a none value..

LGTM

Sep 10 2020, 2:58 AM · Restricted Project

Jun 16 2020

JonasToth added a comment to D81923: [clang-tidy] Add modernize-use-ranges check..

great to see such a check!

Jun 16 2020, 4:58 AM · Restricted Project

Jan 30 2020

JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

This generated 56 "const const" declarations, of which 25 were triple const!

Jan 30 2020, 11:58 AM · Restricted Project, Restricted Project

Jan 25 2020

JonasToth added a comment to D73203: [clang-tidy] Prevent a remove only fixit causing a conflict when enclosed by another fixit.

We need tests for that. There should be tests for the exporting-feature. Maybe they could provide a good starting point on adding tests. (not sure if there is unit-test code for isolated testing, but some tests do exist!)

Jan 25 2020, 12:57 PM · Restricted Project, Restricted Project
JonasToth added a comment to D72553: [clang-tidy] Add performance-prefer-preincrement check.

Yeah, the libc++ failures are not your fault :)
My 2 cents added.

Jan 25 2020, 12:48 PM · Restricted Project, Restricted Project

Jan 20 2020

JonasToth added inline comments to D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved.
Jan 20 2020, 9:02 AM · Restricted Project, Restricted Project

Jan 19 2020

JonasToth added a comment to D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements.

Is this good to land and if so anyone able to commit?

Jan 19 2020, 11:58 PM · Restricted Project, Restricted Project

Jan 13 2020

JonasToth added a comment to D72630: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init.

K. Thank you for fixing it!

Jan 13 2020, 2:11 PM · Restricted Project
JonasToth accepted D72630: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init.

LGTM in general. The linked bug report misses a "4" --> PR44440, so please the description for that.

Jan 13 2020, 1:24 PM · Restricted Project

Jan 11 2020

JonasToth updated subscribers of rG4c48ea68e491: [ASTMatchers] extract public matchers from const-analysis into own patch.

@gkistanova Do you know the toolchain on the power-pc builders? I get an ICE with this patch (see the prior commit) and would like to find out whats the problem. Thanks for your time!

Jan 11 2020, 11:03 AM
JonasToth committed rG23a799adf0ab: Revert "[ASTMatchers] extract public matchers from const-analysis into own… (authored by JonasToth).
Revert "[ASTMatchers] extract public matchers from const-analysis into own…
Jan 11 2020, 10:44 AM
JonasToth added a reverting change for rG4c48ea68e491: [ASTMatchers] extract public matchers from const-analysis into own patch: rG23a799adf0ab: Revert "[ASTMatchers] extract public matchers from const-analysis into own….
Jan 11 2020, 10:44 AM
JonasToth added a comment to rG4c48ea68e491: [ASTMatchers] extract public matchers from const-analysis into own patch.

The powerpc buildbots get an internal compiler error and crash. No Idea why, reverted for now.

Jan 11 2020, 10:44 AM
JonasToth committed rG4c48ea68e491: [ASTMatchers] extract public matchers from const-analysis into own patch (authored by JonasToth).
[ASTMatchers] extract public matchers from const-analysis into own patch
Jan 11 2020, 10:26 AM
JonasToth closed D72505: [ASTMatchers] extract public matchers from const-analysis into own patch.
Jan 11 2020, 10:26 AM · Restricted Project
JonasToth updated the diff for D72505: [ASTMatchers] extract public matchers from const-analysis into own patch.
  • remove merge conflict markers in docs
Jan 11 2020, 10:08 AM · Restricted Project
JonasToth updated the diff for D72505: [ASTMatchers] extract public matchers from const-analysis into own patch.
  • fix compilation error, missing getType
Jan 11 2020, 10:08 AM · Restricted Project
JonasToth updated the diff for D72505: [ASTMatchers] extract public matchers from const-analysis into own patch.
  • add test for K&R functions
Jan 11 2020, 9:51 AM · Restricted Project
JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

Thank you for looking into this; I'll start building diff14 to test locally.

Jan 11 2020, 9:33 AM · Restricted Project, Restricted Project
JonasToth added a comment to D72362: [clang-tidy] misc-no-recursion: a new check.

So that is were the CTU question comes from? :)

Jan 11 2020, 9:33 AM · Restricted Project, Restricted Project
JonasToth added a comment to D72239: [clang-tidy] new opencl recursion not supported check.

The other recursion check seems more sophisticated. What is your take on it? Would you consider it better as well, what is your opinion on it?

Jan 11 2020, 8:57 AM · Restricted Project, Restricted Project, Restricted Project
JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

@0x8000-0000 i did remove dependentTypes from matching. Locally that works with your reported test-case (i was running clang-tidy wrong :().

Jan 11 2020, 8:37 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • fix false positive with ignoring dependentTypes
Jan 11 2020, 8:28 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • remove pointee-analysis artifacts as this will be done later
  • remove unnecessary comments
Jan 11 2020, 8:10 AM · Restricted Project, Restricted Project
JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

Here is a minimal example that shows the problem:

#include <memory>

template<typename SomeValue>
struct DoGooder
{
    DoGooder(void* accessor, SomeValue value)
    {
    }

};

struct Bingus
{
    static constexpr auto someRandomConstant = 99;
};

template<typename Foo>
struct HardWorker
{
    HardWorker()
    {
        const DoGooder anInstanceOf{nullptr, Foo::someRandomConstant};
    }
};

struct TheContainer
{
    std::unique_ptr<HardWorker<Bingus>> m_theOtherInstance;
};

Example run:

$ /opt/clang10/bin/clang-tidy -checks="-*,cppcoreguidelines-const-correctness" -header-filter=".*" reproconst.cpp -- -std=c++17 -Wall
54 warnings generated.
reproconst.cpp:22:9: warning: variable 'anInstanceOf' of type '<dependent type>' can be declared 'const' [cppcoreguidelines-const-correctness]
        const DoGooder anInstanceOf{nullptr, Foo::someRandomConstant};
        ^
                       const
Suppressed 53 warnings (53 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
Jan 11 2020, 2:23 AM · Restricted Project, Restricted Project

Jan 10 2020

JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

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').
Jan 10 2020, 3:20 PM · Restricted Project, Restricted Project
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • 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
Jan 10 2020, 8:48 AM · Restricted Project, Restricted Project
JonasToth updated the diff for D72505: [ASTMatchers] extract public matchers from const-analysis into own patch.
  • address review comments
Jan 10 2020, 8:15 AM · Restricted Project
JonasToth added inline comments to D72505: [ASTMatchers] extract public matchers from const-analysis into own patch.
Jan 10 2020, 8:15 AM · Restricted Project
JonasToth committed rGcdd05f2aea3b: [NFC] format unittest for ExprMutAnalyzer (authored by JonasToth).
[NFC] format unittest for ExprMutAnalyzer
Jan 10 2020, 7:15 AM
JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • fix unit tests for exprmutanalyzer, whitespace was the problem
  • Merge branch 'master' into feature_transform_const.patch
Jan 10 2020, 7:15 AM · Restricted Project, Restricted Project
JonasToth created D72505: [ASTMatchers] extract public matchers from const-analysis into own patch.
Jan 10 2020, 6:55 AM · Restricted Project

Jan 9 2020

JonasToth updated the diff for D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
  • 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 pointers and transform as well, for testing
  • fix canResolveToExpr to account for derivedToBaseClass casts
  • restore unit tests for expr mut analyzer - there was an oversight in my build that lead them not being running
  • short id for matcher
  • fix test for fixed false negative
  • rip out unused matchers that are now unnecessary
  • try to ignore auto-types that are type-dependent
  • consider 'static_cast<Type&>()' always a mutation, speculativly fix tests for that
Jan 9 2020, 11:29 AM · Restricted Project, Restricted Project
JonasToth added a comment to D72438: [clang-tidy] For checker `readability-misleading-indentation` update tests..

I'll point out that with this approach we lose all test coverage of the checker with delayed template parsing on, even though most of the test works in that mode and delayed template parsing is the default when targeting windows.

Jan 9 2020, 5:21 AM · Restricted Project, Restricted Project
JonasToth accepted D72438: [clang-tidy] For checker `readability-misleading-indentation` update tests..

LGTM.
This check already existed in clang-tidy and had a bug. So we should fix that (thank you for that) and probably even backport (release coming soon).
So even though clang itself might be able to do it, clang-tidy should do it properly as well.

Jan 9 2020, 2:26 AM · Restricted Project, Restricted Project

Jan 8 2020

JonasToth added a comment to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.

I did evaluate the performance of the check on LLVM with the script in the attachement.

Jan 8 2020, 4:14 PM · Restricted Project, Restricted Project
JonasToth added inline comments to D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness.
Jan 8 2020, 2:42 PM · Restricted Project, Restricted Project
JonasToth added a comment to D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check..

Adding a -fno-delayed-template-parsing to the RUN-line should suffice.

Jan 8 2020, 2:32 PM · Restricted Project, Restricted Project