[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, EricWF. Add citations to the Coroutines TS to the `isValidCoroutineContext` function, as well as a FIXME and test for [expr.await]p2, which states a co_await expression cannot be used in a default argument. Test Plan: check-

[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Also @GorNishanov I'm curious about your two cents on whether comments like these are valuable. If you think they are I may add a few more with post-commit review. Repository: rC Clang https://reviews.llvm.org/D48519

[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:260 else if (MD && MD->isMoveAssignmentOperator()) return DiagInvalid(DiagMoveAssign); + // [basic.start.main]p3: "The function main shall not be a coroutine." @GorNishanov Is there

[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 152597. modocache added a comment. Great, thanks for the review! I added a reference to N4499. Repository: rC Clang https://reviews.llvm.org/D48519 Files: lib/Sema/SemaCoroutine.cpp test/SemaCXX/coroutines.cpp Index: test/SemaCXX/coroutines.cpp ==

[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC335420: [Sema] isValidCoroutineContext FIXME and citations (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D48519?vs=152597&id=152598#toc Repository: rC Clan

[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 152599. modocache added a comment. Great, thanks @GorNishanov! I moved the 'can throw' logic into a function called 'memberCallExpressionCanThrow', to convey that some dyn_cast'ing is going on. Repository: rC Clang https://reviews.llvm.org/D47673 Fil

[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC335422: [Coroutines] Less IR for noexcept await_resume (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D47673?vs=152599&id=152602#toc Repository: rC Clang h

[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume

2018-06-23 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL335422: [Coroutines] Less IR for noexcept await_resume (authored by modocache, committed by ). Repository: rL LLVM https://reviews.llvm.org/D47673 Files: cfe/trunk/lib/CodeGen/CGCoroutine.cpp cfe/

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

2018-10-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Oh, I'm sorry I let this languish! I'll address your comments later this week, @vsk. Thanks so much for the review! Repository: rC Clang https://reviews.llvm.org/D44672 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

2018-10-29 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: rsmith, rjmccall, doug.gregor, ahatanak. In https://reviews.llvm.org/D45898?id=157373, @rsmith advises to move a `checkMemberDestructor` into `InitListChecker::CheckStructUnionTypes`, to check base classes for an accessible destructor whe

[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

2018-10-29 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Here's a compiler explorer link demonstrating that GCC 8.2 and Clang 7.0.0 compile the example code, but Clang trunk emits an error: https://godbolt.org/z/l3baI_ I realize the proposed "fix" here isn't likely to be exactly correct, but I thought it could be a good st

[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

2018-10-30 Thread Brian Gesiak via Phabricator via cfe-commits
modocache abandoned this revision. modocache added a comment. Yes, thanks @rsmith! And sorry @ahatanak for the trouble of explaining your code here. The new standards behavior does impact @twoh and I's codebase in a few places, but as you explained we can simply change the source to not use li

[PATCH] D56509: [AST] Remove ASTContext from getThisType (NFC)

2019-01-10 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Great, thanks for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56509/new/ https://reviews.llvm.org/D56509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D57032: [SemaCXX] Param diagnostic matches overload logic

2019-01-21 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added a reviewer: rsmith. Given the following test program: class C { public: int A(int a, int& b); }; int C::A(const int a, int b) { return a * b; } Clang would produce an error message that correctly diagnosed the redeclaration of

[PATCH] D57032: [SemaCXX] Param diagnostic matches overload logic

2019-01-30 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 184284. modocache added a comment. Thanks, @cpplearner! You're absolutely right. I got confused because I didn't realize that the method `FunctionProtoType::getUnqualifiedType` was being used from within `Sema::FunctionParamTypesAreEqual`, not `QualType::

[PATCH] D54075: [coroutines] Fix fallthrough warning on try/catch

2018-11-03 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, tks2103, rsmith. Herald added a subscriber: EricWF. The test case added in this diff would incorrectly warn that control flow may fall through without returning. Here's a standalone example: https://godbolt.org/z/dCwXEi The

[PATCH] D54075: [coroutines] Fix fallthrough warning on try/catch

2018-11-03 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Great, thanks! Repository: rC Clang https://reviews.llvm.org/D54075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54075: [coroutines] Fix fallthrough warning on try/catch

2018-11-03 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC346074: [coroutines] Fix fallthrough warning on try/catch (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D54075?vs=172498&id=172508#toc Repository: rC Clang

[PATCH] D53212: inhereit LLVM_ENABLE_LIBXML2

2018-11-05 Thread Brian Gesiak via Phabricator via cfe-commits
modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Sorry to have let this languish! LGTM. Repository: rC Clang https://reviews.llvm.org/D53212 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D51568: [modules] Add `-fno-absolute-module-directory` flag for relocatable modules

2018-11-28 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added subscribers: manmanren, bruno, modocache. modocache added reviewers: manmanren, bruno. modocache added a comment. Herald added a subscriber: arphaman. Friendly ping! Could someone recommend a reviewer for this? Or is there something wrong with the patch? Looking at the blame I ca

[PATCH] D58216: Support attribute used in member funcs of class templates II

2019-03-01 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Friendly ping! @aaron.ballman it looks like you accepted D56928 , could you take a look at this as well? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58216/new/ https://reviews.llvm.org/D58216 _

[PATCH] D58751: Order File Instrumentation: add clang support for -forder-file-instrumentation

2019-03-04 Thread Brian Gesiak via Phabricator via cfe-commits
modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Looks good, thanks for adding a driver option for this! Comment at: include/clang/Driver/Options.td:774 +Group, Flags<[CoreOption]>, +HelpText<"Generate instrum

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

2019-03-04 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: rsmith, andrewjcg. Herald added a project: clang. This patch addresses https://bugs.llvm.org/show_bug.cgi?id=39287. Clang creates a 'std' namespace in one of two ways: either it parses a `namespace std { ... }` declaration, or it creates

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-06 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, tks2103, rsmith. Herald added subscribers: jdoerfert, EricWF. Herald added a project: clang. As reported in https://bugs.llvm.org/show_bug.cgi?id=40978, it's an error to use the `co_yield` or `co_await` keywords outside of a

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-06 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:675 + // Second emphasis of [expr.await]p2: must be outside of an exception handler. + if (S.getCurScope()->getFlags() & Scope::CatchScope) { +S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-07 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:197-200 - if (S.isUnevaluatedContext()) { -S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword; -return false; - } lewissbaker wrote: > Does removing this check now mean

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-07 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 189763. modocache added a comment. Add test case for executing a lambda using co_yield within a catch block. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59076/new/ https://reviews.llvm.org/D59076 Files: include/clang/B

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-07 Thread Brian Gesiak via Phabricator via cfe-commits
modocache marked an inline comment as done. modocache added inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:675 + // Second emphasis of [expr.await]p2: must be outside of an exception handler. + if (S.getCurScope()->getFlags() & Scope::CatchScope) { +S.Diag(Loc, di

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-14 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 190718. modocache added a comment. Thank you for the reviews! This revision fixes the nested try/catch behavior. I still need to modify this such that semantic analysis continues for the rest of the function body. Repository: rC Clang CHANGES SINCE LA

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-14 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 190727. modocache added a comment. Remove unused function parameter. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59076/new/ https://reviews.llvm.org/D59076 Files: include/clang/Basic/DiagnosticSemaKinds.td include/cl

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-14 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 190740. modocache added a comment. OK, I've responded to all your review comments -- thank you! Please give this another look when you get a chance. I would like to emit note diagnostics pointing to the catch blocks, but I've left that as a FIXME for now.

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

2019-03-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. I realize this isn't the correct solution, but would any would-be reviewers like to comment on the problem? Whether it's here or on the Bugzilla report https://bugs.llvm.org/show_bug.cgi?id=39287, as a newcomer to Clang modules I could use some help understanding whet

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-15 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC356296: [coroutines][PR40978] Emit error for co_yield within catch block (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D59076?vs=190740&id=190890#toc Reposit

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Great, thanks for the reviews, everyone! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59076/new/ https://reviews.llvm.org/D59076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-22 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. I'm pushing a revert. Sorry for the trouble! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59076/new/ https://reviews.llvm.org/D59076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-22 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Sorry for the late response, I'm looking now. Should I revert this for now? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59076/new/ https://reviews.llvm.org/D59076 ___ cfe-commits mailing

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

2019-03-22 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Reverted in rC356296 . I'll rework this change along with a test confirming https://bugs.llvm.org/show_bug.cgi?id=41171 doesn't occur. Apologies! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59076/ne

[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, tks2103, rsmith. Herald added subscribers: jdoerfert, EricWF. Herald added a project: clang. https://reviews.llvm.org/D59076 added a new coroutine error that prevented users from using 'co_await' or 'co_yield' within a except

[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: lib/Parse/ParseStmt.cpp:2293 // FIXME: Possible draft standard bug: attribute-specifier should be allowed? StmtResult Block(ParseCompoundStatement()); if (Block.isInvalid()) riccibruno wrote: > Just to make su

[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thank you for the review! Comment at: test/SemaCXX/exceptions.cpp:25 + int ref = k; +} int j = i; // expected-error {{use of undeclared identifier 'i'}} riccibruno wrote: > I am wondering if there is already a test which

[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC356865: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block" (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D59752?vs=192045&id=192050#

[PATCH] D59765: [Lex] Warn about invisible Hangul whitespace

2019-03-25 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: chandlerc, rsmith. Herald added a subscriber: jdoerfert. Herald added a project: clang. On Twitter @LunarLambda pointed out that Clang allows Hangul whitespace Unicode characters in identifiers, which allows users to write very confusing

[PATCH] D59765: [Lex] Warn about invisible Hangul whitespace

2019-03-25 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 192092. modocache added a comment. Remove unneeded change to test identifier 'xx'. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59765/new/ https://reviews.llvm.org/D59765 Files: lib/Lex/Lexer.cpp test/Lexer/unicode.c

[PATCH] D46140: [coroutines] Add std::experimental::task type

2019-03-26 Thread Brian Gesiak via Phabricator via cfe-commits
modocache closed this revision. modocache added a comment. Committed in rL357010 . Apologies, I forgot to include the differential revision in the commit message so this diff wasn't closed automatically as a result. I'll comment on rL357010

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2018-09-26 Thread Brian Gesiak via Phabricator via cfe-commits
modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. This is great, thanks! Sorry for letting it languish. I defer to @GorNishanov, but I don't see why this couldn't go in now and if there're any edge cases I'm missing we can address those

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2018-10-07 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343949: [coro]Pass rvalue reference for named local variable to return_value (authored by modocache, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.or

[PATCH] D57032: [SemaCXX] Param diagnostic matches overload logic

2019-01-31 Thread Brian Gesiak via Phabricator via cfe-commits
modocache marked an inline comment as done. modocache added a comment. Herald added a project: clang. Friendly ping! Is there anything I can do to improve this patch? I think it's a clear improvement of the diagnostic, with a test case to boot! Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D57032: [SemaCXX] Param diagnostic matches overload logic

2019-01-31 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352831: [SemaCXX] Param diagnostic matches overload logic (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D57032?vs=184284&id=184667#toc Repository: rC Clang

[PATCH] D57032: [SemaCXX] Param diagnostic matches overload logic

2019-01-31 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thanks for the reviews! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57032/new/ https://reviews.llvm.org/D57032 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D62550: [coroutines][PR41909] Don't build dependent coroutine statements for generic lambda

2019-06-02 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362348: [coroutines][PR41909] Don't build dependent coroutine statements for generic… (authored by modocache, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Reposit

[PATCH] D62550: [coroutines][PR41909] Don't build dependent coroutine statements for generic lambda

2019-06-02 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Great, thanks for the review! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62550/new/ https://reviews.llvm.org/D62550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D62550: [coroutines][PR41909] Don't build dependent coroutine statements for generic lambda

2019-06-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: cfe/trunk/lib/Sema/TreeTransform.h:7173 +auto *MD = dyn_cast_or_null(FD); +if (!MD || !MD->getParent()->isGenericLambda()) { + assert(!Promise->getType()->isDependentType() && rsmith wrote: > This assert d

[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

[PATCH] D65044: [Format] Add option to enable coroutines keywords

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 subscriber: EricWF. Herald added a project: clang. In a previous change, https://reviews.llvm.org/D65043, I allowed users to explicitly opt-in to formatting according to the C++20 standard.

[PATCH] D65149: [Format] Add test demonstrating PR42722

2019-07-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: ank, klimek, acoomans. Herald added a project: clang. https://bugs.llvm.org/show_bug.cgi?id=42722 describes what I believe to be a bug in lambda formatting. If it is indeed a bug, I'd like to commit this test that reliably reproduces it.

[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 I'

[PATCH] D65183: [Format] Make it easy to add new format::FormatStyle::LanguageStandard. NFCI

2019-07-23 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. LGTM, but I don't actively work in this codebase so I really can't say. I'll wait to hear from some other more active clang-format reviewers. Comment at: lib/Format/TokenAnnotator.cpp:2862 return Right.is(TT_TemplateCloser) && Left.is(TT_Templat

[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 in

[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; + LangO

[PATCH] D65149: [Format] Add test demonstrating PR42722

2019-07-26 Thread Brian Gesiak via Phabricator via cfe-commits
modocache marked an inline comment as done. modocache added a comment. Sure thing! Just to be clear: this test doesn't fail, it passes. My intention was to commit this, then commit a patch that improved the indentation behavior, which would also include a change to the test that demonstrated the

[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 se

[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-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 Githu

[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 htt

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

2019-08-09 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 214370. modocache added a comment. Thanks for the review, @vsk! Sorry it took me so long to update this diff. In the mailing list discussion, http://lists.llvm.org/pipermail/llvm-dev/2018-March/121925.html, you mentioned that I should use an allow-list of

[PATCH] D65149: [Format] Add test demonstrating PR42722

2019-08-11 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5184 + "c++;\n" + "d++\n" + " });\n" MyDeveloperDay wrote: > modocache wrote: > > This is a passing test that demonstrates that t

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

2019-08-13 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368675: [CodeGen] Disable UBSan for coroutine functions (authored by modocache, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

2019-08-13 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thank you! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44672/new/ https://reviews.llvm.org/D44672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mai

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

2019-05-13 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thanks @rsmith for the guidance here! I appreciate it very much. One snag I ran into after following your suggestion, though, is that when I modify `ASTDeclReader::findExisting` to return Sema's existing implicit std namespace, I run into an assertion later on, when t

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

2019-05-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 199708. modocache added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Thanks for the help, @rsmith! Your suggestions were spot-on. (It took me a little while to figure out why, even using the `LazyDeclPtr` directly, I was

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

2019-05-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 199709. modocache added a comment. Oops, sent the patch from the wrong repository. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58920/new/ https://reviews.llvm.org/D58920 Files: lib/Serialization/ASTReaderDecl.cpp tes

[PATCH] D62035: [AST] const-ify ObjC inherited class search

2019-05-16 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added a reviewer: rjmccall. Herald added a project: clang. When writing an AST matcher to find inherited Objective-C classes, I noticed the returned `ObjCInterfaceDecl*` was mutable. It doesn't seem like it needs to be mutable, so this patch makes it cons

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

2019-05-21 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 200518. modocache added a comment. Hmm... alright, I'm not really sure how I could implement a test that fails without this, but I added a check in the FindExistingResult destructor. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

2019-05-28 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. @rsmith, what do you think of the patch as-is? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58920/new/ https://reviews.llvm.org/D58920 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D62550: [coroutines][PR41909] Don't build dependent coroutine statements for generic lambda

2019-05-28 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, EricWF, lewissbaker, tks2103. Herald added a project: clang. https://bugs.llvm.org/show_bug.cgi?id=41909 describes an issue in which a generic lambda that takes a dependent argument `auto set` causes the template instantiatio

[PATCH] D56243: [coroutines] Experimenting with __builtin_coro_frame_max_size

2019-01-02 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, lewissbaker, tks2103. Herald added subscribers: cfe-commits, kristina, EricWF. This commit implements a proposed addendum to the C++ Coroutines TS that would allow users to query, at compile time, the maximum potential size o

[PATCH] D56271: [SemaCXX] Fix ICE for unexpanded parameter pack

2019-01-03 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added a reviewer: rsmith. The documentation for RecursiveASTVisitor::TraverseDecl states that the Decl being traversed may be null. In fact, this is the case when a CXXCatchStmt with no exception decl is traversed. Because the visitor for diagnosing unexp

[PATCH] D56271: [SemaCXX] Fix ICE for unexpanded parameter pack

2019-01-06 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL350501: [SemaCXX] Fix ICE for unexpanded parameter pack (authored by modocache, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D56271: [SemaCXX] Fix ICE for unexpanded parameter pack

2019-01-06 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thank you for the review! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56271/new/ https://reviews.llvm.org/D56271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D56509: [AST] Remove ASTContext from getThisType (NFC)

2019-01-09 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: akyrtzi, mikael. Herald added subscribers: dexonsmith, mehdi_amini. https://reviews.llvm.org/D54862 removed the usages of `ASTContext&` from within the `CXXMethodDecl::getThisType` method. Remove the parameter altogether, as well as all u

[PATCH] D56243: [coroutines] Experimenting with __builtin_coro_frame_max_size

2019-01-09 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 180903. modocache added a comment. Thanks for the offline review @GorNishanov! This revision allows constexpr usages of __builtin_coro_frame_max_size. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56243/new/ https://review

[PATCH] D56243: [coroutines] Experimenting with __builtin_coro_frame_max_size

2019-01-09 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 180905. modocache added a comment. Remove obsoleted code I accidentally included. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56243/new/ https://reviews.llvm.org/D56243 Files: include/clang/AST/ASTContext.h include/c

[PATCH] D56509: [AST] Remove ASTContext from getThisType (NFC)

2019-01-10 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC350914: [AST] Remove ASTContext from getThisType (NFC) (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D56509?vs=180893&id=181200#toc Repository: rC Clang C

[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] D65044: [Format] Add option to enable coroutines keywords

2019-09-13 Thread Brian Gesiak via Phabricator via cfe-commits
modocache abandoned this revision. modocache added a comment. I work on a C++17 codebase with coroutines enabled, but yes just formatting the codebase as if it were C++20 seems fine. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65044/new/

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

2018-04-19 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, EricWF. http://wg21.link/P0664r2 section "Evolution/Core Issues 24" describes a proposed change to Coroutines TS that would have any exceptions thrown after the initial suspend point of a coroutine be caught by the handler sp

[PATCH] D41980: Add tests for llvm-bcanalyzer stream types

2018-04-21 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 143458. modocache added a comment. Herald added a reviewer: george.karpenkov. Added `-fmodule-format=raw`. Repository: rC Clang https://reviews.llvm.org/D41980 Files: test/Misc/serialized-diags-bcanalyzer.c test/PCH/include-stream-type.cpp Index:

[PATCH] D41980: Add tests for llvm-bcanalyzer stream types

2018-04-21 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC330530: Add tests for llvm-bcanalyzer stream types (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D41980?vs=143458&id=143464#toc Repository: rC Clang https

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

2018-05-02 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 144908. modocache added a comment. Thanks for the review, @GorNishanov. Here's a more correct solution: an i1 is used to keep track of whether await_resume threw. If it did, the coroutine body is skipped, and we go straight to the final suspend point. Othe

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

2018-05-02 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 144961. modocache added a comment. Oops, thanks for testing on release mode, @GorNishanov. Turns out I had a dangling pointer. With this update the tests pass on both release and debug. Repository: rC Clang https://reviews.llvm.org/D45860 Files: lib

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

2018-05-04 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC331519: [Coroutines] Catch exceptions in await_resume (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D45860?vs=144961&id=145188#toc Repository: rC Clang ht

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

2018-05-04 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thanks again for all the reviews, @GorNishanov! Very much appreciated. Comment at: lib/CodeGen/CGCoroutine.cpp:220 CGF.EmitBlock(ReadyBlock); + CXXTryStmt *TryStmt = nullptr; + if (Coro.ExceptionHandler && Kind == AwaitKind::Init) { -

[PATCH] D48863: [Sema] Explain coroutine_traits template in diag

2018-07-02 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, EricWF. If a user defines a coroutine_traits type that takes an incorrect number of template parameters, or for some reason they include such a type in their program, they receive a cryptic error message: "too few template ar

[PATCH] D48863: [Sema] Explain coroutine_traits template in diag

2018-07-03 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 153956. modocache added a comment. Oops, apologies, I included a line I shouldn't have in the previous diff. Repository: rC Clang https://reviews.llvm.org/D48863 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaCoroutine.cpp test/Se

[PATCH] D48863: [Sema] Explain coroutine_traits template in diag

2018-07-03 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9053 "a coroutine">; +def note_coroutine_types_for_traits_here : Note< + "the coroutine traits class template is being instantiated using the return " GorNishanov wrote: >

[PATCH] D48981: Add caching when looking up coroutine_traits

2018-07-05 Thread Brian Gesiak via Phabricator via cfe-commits
modocache resigned from this revision. modocache added a comment. This LGTM but I'll just wait for @GorNishanov to accept the patch, just in case I'm missing something. I'd be happy to commit this for you once Gor accepts! :) Repository: rC Clang https://reviews.llvm.org/D48981 __

[PATCH] D49099: Remove qualtype qualifier in coroutine error to prevent assert in debug

2018-07-10 Thread Brian Gesiak via Phabricator via cfe-commits
modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Excellent, thanks! Repository: rC Clang https://reviews.llvm.org/D49099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D49099: Remove qualtype qualifier in coroutine error to prevent assert in debug

2018-07-10 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. If you don't have commit access, let me know here if you'd like me to commit this on your behalf. Repository: rC Clang https://reviews.llvm.org/D49099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D49099: Remove qualtype qualifier in coroutine error to prevent assert in debug

2018-07-10 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC336748: Remove qualtype qualifier in coroutine error to prevent assert in debug (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D49099?vs=154692&id=154910#toc

[PATCH] D48981: Add caching when looking up coroutine_traits

2018-07-14 Thread Brian Gesiak via Phabricator via cfe-commits
modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Yup, LGTM! I'll land this now. Repository: rC Clang https://reviews.llvm.org/D48981 ___ cfe-commits mailing list cfe-commits@lists.llvm.

<    1   2   3   4   >