[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd97025ad3a70: [clang-format][NFC] Fix a bug in

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I agree performance of this if is unlikely to be a game changer performance wise Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116318/new/ https://reviews.llvm.org/D116318

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

owenpan added a comment. In D116318#3212351 , @curdeius wrote: > I'm in the same position as @hazardyknusperkeks. > If you need something to simplify the code you can add a helper > `getPreviousTokenOrNull` or something like that in your patch. > But we

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

curdeius added a comment. On another note, you can fix the comment typo without a review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116318/new/ https://reviews.llvm.org/D116318 ___ cfe-commits

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

curdeius added a comment. I'm in the same position as @hazardyknusperkeks. If you need something to simplify the code you can add a helper `getPreviousTokenOrNull` or something like that in your patch. But we certainly don't want to pay for the `if` check each time we call `getPreviousToken`.

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

owenpan added a comment. In D116318#3211689 , @HazardyKnusperkeks wrote: > I'm against that patch. > >> Don't pay for what you don't use. > > We should not put the size check into every call. That's what you must do now **without **this patch, which

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

HazardyKnusperkeks added a comment. I'm against that patch. > Don't pay for what you don't use. We should not put the size check into every call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116318/new/ https://reviews.llvm.org/D116318

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

owenpan added a comment. In D116318#3211258 , @curdeius wrote: > Wasn't assert enough? Could you add a test with code that provokes the > problem? This fix is an NFC, so there will be no (user) test case for it. :) The current behavior does not match

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Wasn't assert enough? Could you add a test with code that provokes the problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, curdeius. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo