[PATCH] D65043: [Format] Add C++20 standard to style options

2019-09-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D65043#1669410 , @sammccall wrote: > (For actual coroutine support, treating `co_return` and `co_yield` like > `return` everywhere might make sense) I agree, with one nit. `co_return` should be treated like `return`, but

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-09-13 Thread Brian Gesiak via Phabricator via cfe-commits
modocache abandoned this revision. modocache added a comment. Oh, thank you! Yes, I had been meaning to abandon this and my other patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65043/new/ https://reviews.llvm.org/D65043

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-09-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Not sure if this patch is still live? I implemented the suggestion from the last round in D67541 . I didn't add the coroutine tests, as clang-format doesn't seem to get any coroutine cases right in 20 mode that it missed in 17 mode.

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-08-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Sorry for taking so long here, I've been swamped. Unfortunately there's a couple of pain points still: - this change is (mostly?) about being able to turn off c++20 parsing, so you preserve old desirable formatting of c++17 code. But the tests don't contain any such

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-08-08 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13780 + // In C++2a, it is interpreted as a prefix increment on 'i'. + verifyFormat("co_yield++ i;"); + verifyFormat("co_yield ++i;", Cpp2a); Quuxplusone wrote: > My comment

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-08-08 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 214168. modocache marked 3 inline comments as done. modocache added a comment. Thanks for the review, @Quuxplusone! I addressed your test comments, but the 'co_yield' test is something I think I'll need to deal with separately. Repository: rG LLVM

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-08-07 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. FWIW, looks unobjectionable to me, except for some nitpicks on the test cases (which are easy to fix so I'm hoping you just will :)). Comment at: clang/unittests/Format/FormatTest.cpp:13780 + // In C++2a, it is interpreted as a prefix increment

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-08-07 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Friendly ping! @sammccall is this what you had in mind? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65043/new/ https://reviews.llvm.org/D65043 ___ cfe-commits mailing list

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-08-05 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 213360. modocache added a comment. Thanks for the reviews, @sammccall, @Quuxplusone, and @MyDeveloperDay. I added C++14 and C++17 options. In an earlier comment I mentioned splitting this work up into a series of commits, but it ended up being a smaller

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-26 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13815 + verifyFormat("co_yield++ i;"); + verifyFormat("co_yield ++i;", Cpp20); + modocache wrote: > Quuxplusone wrote: > > If you're going to test C++11's behavior here,

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-26 Thread Brian Gesiak via Phabricator via cfe-commits
modocache marked 2 inline comments as done. modocache added inline comments. Comment at: clang/lib/Format/Format.cpp:2373 LangOpts.CPlusPlus17 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1; - LangOpts.CPlusPlus2a = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1; +

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D65043#1599220 , @sammccall wrote: > Clang actually defaults to c++14 since clang 6. (CompilerInvocation.cpp near > `CLANG_DEFAULT_STD_CXX`) Today I learned. Thanks! :) > It's important that clang-format works sensibly

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-24 Thread Brian Gesiak via Phabricator via cfe-commits
modocache marked 3 inline comments as done. modocache added a comment. > It sounds like currently they're very different, and you're proposing to make > them basically the same. I think that's a good thing. I 100% agree with this statement, and thank you @sammccall for the suggestion to move

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:76 +IO.enumCase(Value, "C++20", FormatStyle::LS_Cpp20); IO.enumCase(Value, "Auto", FormatStyle::LS_Auto); } Prior to C++20 being confirmed and given the line 2373 being

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D65043#1599148 , @Quuxplusone wrote: > Peanut gallery says: A priori, I don't see any reason for clang-format's > `LanguageStandard` options to diverge from Clang's own `-std=` options. It > sounds like currently they're

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. Peanut gallery says: A priori, I don't see any reason for clang-format's `LanguageStandard` options to diverge from Clang's own `-std=` options. It sounds like currently they're very different, and you're proposing to make them basically the same. I think that's a

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. So there's precedent for this in rL185149 (D1028 ). @klimek may have historical context. Overall, this makes sense. Since the coroutines flag flip landed on the 9.x release branch, we may want this

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Friendly ping! I'm wondering, from the clang-format maintainers' point of view, whether a diff like this and https://reviews.llvm.org/D65044 make sense to add? I do think that it is useful for users to specify whether they wish to use C++11 or C++20 constructs, but

[PATCH] D65043: [Format] Add C++20 standard to style options

2019-07-20 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: rsmith, sammccall, Typz, klimek. Herald added a project: clang. When C++ coroutines were adopted as part of the C++20 standard, a change was committed in https://github.com/llvm/llvm-project/commit/10ab78e854f: coroutine keywords such as