[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter
Björn Schäpers via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 5 03:48:19 PST 2022
HazardyKnusperkeks reopened this revision.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.
In D115060#3219116 <https://reviews.llvm.org/D115060#3219116>, @curdeius wrote:
> @HazardyKnusperkeks, it *seems* as if this commit (or one of others indicated on Changes tab of the link below) provoked failures in ASan. But it could be something else.
> https://lab.llvm.org/buildbot/#/builders/5/builds/16734
> Could you have a look?
Okay, I don't have access to ASAN or so, I'm using MinGW on Windows. But I suspect it is the Assignment of the `PreviousLine` since this is not existent every time. So I see the following solutions:
1. Only name `NextLine`, and use `I[-1]`.
2. `const auto HasPreviousLine = I != AnnotatedLines.begin(); const auto &PreviousLine = HasPreviousLine ? *I[-1] : *I;` which is safe, since `PreviousLine` is only used if `HasPreviousLine` is true, but is a bit confusing. It would get an explaining comment.
3. Rearrange the statements so that we can have only one check if there is a previous line and define `PreviousLine` inside that `if`. It remains to be seen if that's NFC.
I would prefer option 3, but if that would change the behavior would go for option 2.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115060/new/
https://reviews.llvm.org/D115060
More information about the cfe-commits
mailing list