- User Since
- Feb 5 2017, 1:32 PM (189 w, 4 d)
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 preferably if you provide me some branch somewhere (github?). I have trouble applying the latest diff to latest master myself.
Thu, Sep 3
Jul 30 2020
Addressed findings by @njames93
Jul 29 2020
The reason was that I thought that line doesn't do anything.
Of course "not crashing" is a valid expectation. If nothing else, then "not crashing" should indeed be tested.
I'll try to improve this ancient diff.
Last change was over 3 months ago. Should this be merged or abandoned?
It doesn't solve everything. It doesn't improve everything. But it's a fix to two specific bugs.
Is someone missing as a reviewer to get this accepted/rejected any faster?
Sorry, but I have to ask: Is there no interest in improving clang-tidy?
Don't get me wrong. I know it's mostly volunteered time. That's also why this ping took almost 2 months for me.
I frankly lost interest in contributing as I have trouble getting any patches included - and I don't have that much free time anymore. Obviously the time issue is more important, but the other one is still a problem.
Jun 2 2020
Apr 24 2020
Mailing list was clear about AST having to many users. No change to clang AST.
Will hopefully be resolved by D75813 instead.
Apr 11 2020
- Add test cases exactly as in bug reports.
- Test case from https://bugs.llvm.org/show_bug.cgi?id=43808 triggered a second bug due to the short parameter names used. Removed incorrect quick exit in SimpleTypoCorrector (MinPossibleEditDistance).
- The two requested cases don't work since 'aaab' is as close to 'aaa' as it is to 'aab' (one char). Nothing is changed as there is no best match available. Instead two new test cases with aaab, aaac and aaad added (test3to3_allAlmost + test3to3_allAlmost_rev).
Apr 10 2020
I need to have a look why the mentioned test case is failing. I'll also add the two examples you provided to the next upload.
Apr 1 2020
I have an opinion on NullStmt, but I would rather see this merged ;-)
Behavior is same as before.
Mar 23 2020
More tests added to prevent further regressions. It's quite difficult to figure out tests which might fail, but now there is a whole bunch of them so it looks good.
Added explicit references from tests to the two bug reports.
Mar 21 2020
Thanks for the feedback!!
Mar 19 2020
Just new tests, easy to review :-)
new test case as requested (line 256)
Mar 18 2020
Add further tests from more bug reports.
Mar 17 2020
Mar 16 2020
Accidentally reverted test file renaming in last commit. Fixed now.
Rebased on current master.
Mar 14 2020
One more thought: This originated for me from attempting to fix braces-around-statements which has quite some trouble finding the correct place to place the }. However this is a good definition of "end location of statement" isn't it? The place where you can end the scope. The place where you could place the next statement!
Thanks for the feedback. I was trying to start a discussion on cfe-dev where I referenced this commit, but I'm also fine having it here! There you'll also find some rationale. Just worried it will get out of sync.
Mar 12 2020
Fine for me. Fixes newline bug in https://bugs.llvm.org/show_bug.cgi?id=45150.
However I don't have "review privileges" here.
Unfortunately I cannot say whether the code is good, but the fix works and certainly helps readability-braces-around-statements which can sometimes add multiple } at the same offset to close several scopes.
Mar 11 2020
Mar 10 2020
Rebased. Solves the svn issue as it's gone in the meantime.
Mar 9 2020
Mar 7 2020
- Extended Test for another macro scenario.
- Added graceful termination to handle problematic locations.
- Changed Lexer::findNextToken to no longer run into that graceful termination. Test is passing. I'm not sure whether that's a good idea? check-clang and check-clang-tools are passing with this change.
Oh sorry. It's crashing. I'll look into it.
Feb 29 2020
Here is the r_brace use case:
Feb 23 2020
Slightly more code changed, assuming it's better understandable now. Review findings incorporated.
Feb 20 2020
Feb 19 2020
Could someone commit this as I cannot? Thanks.
Alexander Lanin <firstname.lastname@example.org>
Feb 18 2020
Feb 9 2020
This was an accident on github. Can/should be closed/deleted here. Sorry!
The proper submission is at https://reviews.llvm.org/D74299
I tested this in WSL1 with ninja check-clang-tools.
Is there anything else I can do (besides setting up a second OS)?
On second thought maybe this should be fixed in clang-tidy and not here?
Maybe besides -export-fixes there should be an -export-warnings or just -yaml-export?
Feb 3 2020
@aaron.ballman I was ready to complain, but you are right. It would make absolutely no sense to move the const, it's just where it should be.
Feb 2 2020
Feb 1 2020
This is now detected by cppcoreguidelines-macro-usage, so this seems out of date.
Review findings applied (no real measurable difference in speed, maybe 100ms) but it's indeed more readable.
Without this fix the export took 12.96 seconds, with this patch 11.07 seconds.
Difference is even bigger when there are less auto fixable findings (70% with FIX-IT in my example).
Jan 26 2020
Jan 22 2020
Updated misc-misplaced-const.c to reflect new output and fixed one wrong col in misc-misplaced-const.cpp - I really do not understand how that happened.
Rebased and verified with check-clang-tools.
Could someone commit this? As I can not.
Alexander Lanin <email@example.com>
Jan 16 2020
Thanks for the review!
Jan 15 2020
Hi, could anyone provide feedback here?
Windows and RC Versions of git do not work with format-patch.
Jan 14 2020
Important: I'm not a python expert. It works fine as far as I can tell. I would feel better if someone with more than a day python experience would say that it is fine.
Jan 12 2020
Updated revision with llvm_unreachable
Jan 8 2020
I guess it's not so easy to fix as you reported this 1,5 years ago. That's an even better reason to update the documentation to work around the issue.
Jan 7 2020
The only difference is the git version in the end:
looking at this at least many are indeed valid results:
I guess false positives could be reduced by eliminating those cases where the variables are (intentionally) modified.
ping? Could someone push this please.
Jan 3 2020
Getting Started since it's Getting Started with the LLVM System
Jan 2 2020
Not sure we can call this updated proposal 'love': it's definitely not the goal to have less documentation, just less redundancy.
I do not have commit rights to the repository, someone please commit this change.
Alexander Lanin <firstname.lastname@example.org>
@Jim git commit --amend --author="Alexander Lanin <email@example.com>"
Jan 1 2020
Removed change in hacking page as discussed.
Can someone commit this as apparently I cannot do it myself (I'll create a PR with updated getting started documentation later...), thanks.
Dec 30 2019
I don't see the error message you mentioned anywhere, but I'm gonna assume it involves the number of surrounding lines. Here is another attempt with
git show HEAD -U999999 > mypatch.patch
Dec 29 2019
Feb 14 2017
Applied review comments and added test cases regarding parenthesis, floats, doubles, wide chars etc
Thanks for the feedback. As I'm new to this I cannot say whether checking only the file in question fits better with clang-tidy’s policy or not.
Also, I’m not sure about ODR. Of course, it’s a possibility, but isn’t the programmer responsible for that? This will be more of an issue as soon as this check provides a Fix-It solution.
Feb 11 2017
Fixes as reported in review
Not sure about CppCoreGuidelines as several guidelines have the same rule and I only used CppCoreGuidelines as it's convenient to link a specific rule. But I can move it if you like?!