Anyway based on the approach above I found two additional regressions. Testing with real code seems to be kind of important ;-)

• Reported column numbers have been changed. Apparently this cannot be checked by expected-warning and expected-note, so I added another FileCheck pass with a few select test cases.

Maybe first look why the columns have changed and whether it's a real issue.

• All warnings are reported first and then all the notes. Apparently this is not checked by expected-warning and expected-note. This will probably also require to be checked by FileCheck directly.

AFAIK other diagnostic tests don't test the in which the diagnostics are emitted. So I wonder whether this is really required.

I think if would be first good to figure out why this changes and then see whether it's a real issue. If it's an issue it needs to be fixed, else the separate patches would be preferred.

I cannot reproduce the problem and there is just not enough info to go on.
Is there any way to get anything in addition? e.g. run the same test with -vv or add some additional print commands?
Maybe it's some access rights problem, or different version of some non obvious dependency or something like that.
Maybe it's as trivial as having to use double quotes for -checks='-*,bugprone-sizeof-container,modernize-use-auto'.
It seems run-clang-tidy exits with code 2, but there is no way it would do that. So I'm totally lost.
http://lab.llvm.org:8011/#/builders/109/builds/690/steps/6/logs/FAIL__Clang_Tools__run-clang-tidy_export-diagnosti

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

I'm not convinced the new approach is better in all cases, so I'm somewhat concerned about regressions from the current behaviour. Did you test this patch on a codebase to see the difference between the previous and new behaviour?

There are no warnings in llvm or ccache, all I found is a low quality codebase at work which does produce 1007 unique Wdocumentation warnings (see wc command).

Readd removed 'print help without crashing' test

Added one missing expect statement in this revision.

@JonasToth: Cannot add assert. EndLoc is indeed invalid in case BeginLoc is outside the macro and EndLoc would be inside the macro. I've added a comment explaining that.

@JonasToth I fully realize that it's voluntary and I do not have a solution. I have the same problem in a different area. Not sure what shall become of some OSS projects when it takes that long to get a patch in. But that's a different discussion.

Rebased and stored at https://github.com/AlexanderLanin/llvm-project/tree/Wdocumentation-fix-its so I have a branch next time I need to rebase :-)

Rebased and stored at https://github.com/llvm/llvm-project/compare/9d6d4b07a...AlexanderLanin:readability-braces-around-statements?expand=1 so I have a branch next time I need to rebase :-)

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

"Minimal" example for the behavior I described before.

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.

Observed behavior:

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.