HazardyKnusperkeks added a comment. In D115060#3224176 <https://reviews.llvm.org/D115060#3224176>, @owenpan wrote:
>> But I suspect it is the Assignment of the `PreviousLine` since this is not >> existent every time. > > Yep! > >> 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. > > There is option 4: change the type of `PreviousLine` to a pointer, and > replace `PreviousLine.` with `PreviousLine->`. (You can rename `PreviousLine` > if you think the naming is inconsistent with `NextLine`.) This would keep the > patch being NFC. I'm open for that, but I think the current diff (plus corrected formatting) is better. And at least according to our tests it is NFC. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115060/new/ https://reviews.llvm.org/D115060 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
