Addressed findings by @njames93

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.

*ping*
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?

*ping*
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.

*ping*

*ping*

*ping*

Mailing list was clear about AST having to many users. No change to clang AST.
Will hopefully be resolved by D75813 instead.

*ping*

*ping*

*ping*

• 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).

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.

I have an opinion on NullStmt, but I would rather see this merged ;-)
Behavior is same as before.

*ping*

*ping*

*ping*

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.

Thanks for the feedback!!

Just new tests, easy to review :-)

new test case as requested (line 256)

Add further tests from more bug reports.

Accidentally reverted test file renaming in last commit. Fixed now.
Rebased on current master.

• Added GotoStmt and SEHLeaveStmt as learned from D76108.
• After applying D76037 and D76054 I was able to run-clang-tidy on llvm and therefore found some new macro related issues

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!

*ping*

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.
Compare http://lists.llvm.org/pipermail/cfe-dev/2020-March/064861.html

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.

Rebased. Solves the svn issue as it's gone in the meantime.

*ping*

*ping*

1. Extended Test for another macro scenario.
2. Added graceful termination to handle problematic locations.
3. 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.

Here is the r_brace use case:

Slightly more code changed, assuming it's better understandable now. Review findings incorporated.

Could someone commit this as I cannot? Thanks.
Alexander Lanin <alex@lanin.de>

*ping*

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?

@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.

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).

In D54943#1815772, @0x8000-0000 wrote:

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

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 <alex@lanin.de>

Thanks for the review!

Hi, could anyone provide feedback here?
Windows and RC Versions of git do not work with format-patch.

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.

Updated revision with llvm_unreachable

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.

The only difference is the git version in the end:

looking at this at least many are indeed valid results:

ping? Could someone push this please.

In D72057#1802982, @aaron.ballman wrote:

LGTM with a small typo fix. Do you need someone to commit this on your behalf?

Getting Started since it's Getting Started with the LLVM System

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 <alex@lanin.de>

@Jim git commit --amend --author="Alexander Lanin <alex@lanin.de>"

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.

Hi,
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

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.

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?!