[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-07-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. For the purposes of not checking a style we are not going to use and because I guess it might make clang-format startup, ever so slightly faster, I think its a reasonable change that likely does no harm. Repository: rG LL

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > I see little value in not checking FallbackStyle (even if it's not used). note that this patch doesn't disable fallbackstyle checking, it is still checked, but not eagerly. > However, I do see value in an early warning (error) on an incorrect style > that can pop up

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I return the question, are there any workflows that would benefit from the proposed behaviour? Apart from those that have already a faulty config? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95538/new/ https://reviews.l

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I see little value in not checking FallbackStyle (even if it's not used). However, I do see value in an early warning (error) on an incorrect style that can pop up later if not caught soon enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D95538#2527335 , @curdeius wrote: > Why is fixing (or removing) the fallback style in the .clang-format file not > enough?? sure that would be one way to go, and that's what the user did already. It feels confusing to me that

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Why is fixing (or removing) the fallback style in the .clang-format file not enough?? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95538/new/ https://reviews.llvm.org/D95538 _

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 319765. kadircet added a comment. - Add tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95538/new/ https://reviews.llvm.org/D95538 Files: clang/lib/Format/Format.cpp clang/unittests/Format/FormatTest

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Sure adding tests, sorry for leaving them out the first time. To give more insights, `format::getStyle()` tries to figure out `FallbackStyle` at the beginning of the function, and errors out if it can't. But in most cases `FallbackStyle` is not needed, e.g. there's alr

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. This needs a unit test, I personally don't understand what this is doing or why its needed, if its a bug can you reference a bug in bugzilla that explains the bug a l

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: MyDeveloperDay, curdeius. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently getStyle() fails immediately if FallbackStyle is not a predefined style, even when it is