[PATCH] D50410: Removing -debug-info-macros from option suggestions test

2018-08-15 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. I don't have access to the PS4 SDK, but this is the most plausible explanation I've seen for why I was experiencing issues on these platforms. Thanks for this! Do you have commit access

[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

2018-05-28 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. Great! Thanks @GorNishanov! https://reviews.llvm.org/D47454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D51333: Diagnose likely typos in include statements

2018-08-28 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added subscribers: aaron.ballman, erikjv, modocache. modocache added reviewers: aaron.ballman, erikjv. modocache added a comment. This looks good to me, but maybe some people who've modified this part of the codebase before could review this as well? @aaron.ballman added a fix-it for a

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

2018-09-03 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. Thanks! I'll land https://reviews.llvm.org/D50410, then this, and monitor the buildbots to see what happens. If they fail again I may revert this once again. Thanks again for looking int

[PATCH] D50410: Removing -debug-info-macros from option suggestions test

2018-09-03 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL341327: Removing -debug-info-macros from option suggestions test (authored by modocache, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50410

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

2018-09-03 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL341329: Re-push "[Option] Fix PR37006 prefix choice in findNearest" (authored by modocache, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

2018-09-03 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Excellent, I think pushing this along with https://reviews.llvm.org/D50410 revealed the true error, as an MSAN buildbot tells use there's a use of an uninitialized value: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/23176/steps/check-clang%20ms

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-06 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 133129. modocache added a comment. Prevent coroutine parameter stores from being moved, rely on https://reviews.llvm.org/D43000 instead. Repository: rC Clang https://reviews.llvm.org/D42606 Files: lib/Sema/SemaCoroutine.cpp test/CodeGenCoroutines/

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-06 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 133154. modocache added a comment. Update the tests to work with scalar parameter copies. Repository: rC Clang https://reviews.llvm.org/D42606 Files: lib/Sema/SemaCoroutine.cpp test/CodeGenCoroutines/coro-alloc.cpp test/CodeGenCoroutines/coro-gro

[PATCH] D36492: [time-report] Add preprocessor timer

2018-02-14 Thread Brian Gesiak via Phabricator via cfe-commits
modocache abandoned this revision. modocache added inline comments. Comment at: include/clang/Lex/PreprocessorOptions.h:165 public: - PreprocessorOptions() : UsePredefines(true), DetailedRecord(false), + PreprocessorOptions() : PPTimer("preprocessor", "Preprocessing"), +

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-14 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 134304. modocache added a comment. Thanks for the review, @GorNishanov, and for pointing out that part of the standard! I added a reference to the implicit object parameter, as well as some tests for that behavior. And thanks for pointing out the flaw in

[PATCH] D42605: [Sema] Toggle diags when finding allocators (NFCI)

2018-02-15 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC325288: [Sema] Toggle diags when finding allocators (NFCI) (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D42605?vs=131728&id=134489#toc Repository: rC Clan

[PATCH] D42605: [Sema] Toggle diags when finding allocators (NFCI)

2018-02-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thanks again, @GorNishanov, for all the reviews! Repository: rC Clang https://reviews.llvm.org/D42605 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-15 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC325291: [Coroutines] Use allocator overload when available (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D42606?vs=134304&id=134495#toc Repository: rC Clan

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Hooray, `[dcl.fct.def.coroutine]/7` is *in!* Thank you for all the reviews, @GorNishanov! Repository: rC Clang https://reviews.llvm.org/D42606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile

2017-06-10 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. Prior to this change, using `-fdiagnostics-show-hotness` with a sampling profile specified via `-fprofile-sample-use=` would result in the Clang frontend emitting a warning: "argument '-fdiagnostics-show-hotness' requires profile-guided optimization information". O

[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile

2017-06-10 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments. Comment at: test/Frontend/optimization-remark-with-hotness.c:32 +// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness 2>&1 | FileCheck \ +// RUN: -check-prefix=PGO_ENABLED %s +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main

[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile

2017-06-11 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 102153. modocache added a comment. Thanks for the suggestions! I moved the sampling test close to the instrumented one, and adjusted bar and foo entry count in the hopes og getting the remarks to include hotness. No luck, however -- the test currently fail

[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile

2017-06-22 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 103647. modocache added a comment. Update the sampling profile text so that it produces the hotness expected by the test. This is ready to go :) https://reviews.llvm.org/D34082 Files: lib/Frontend/CompilerInvocation.cpp test/Frontend/Inputs/optimizat

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

2018-01-04 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: yamaguchi, v.g.vassilev, teemperor, ruiu. Depends on https://reviews.llvm.org/D41732. Utilities such as `opt`, when invoked with arguments that are very nearly spelled correctly, suggest the correctly spelled options: bin/opt -hel o

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

2018-01-04 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thank you, @bruno! Good idea, I'll add a `-cc1` invocation test. Comment at: lib/Driver/Driver.cpp:191 if (A->getOption().hasFlag(options::Unsupported)) { - Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args); - ContainsError |=

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

2018-01-04 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 128681. modocache added a comment. Add 'did you mean' suggestions for -cc1 invocations as well. Repository: rC Clang https://reviews.llvm.org/D41733 Files: include/clang/Basic/DiagnosticDriverKinds.td lib/Driver/Driver.cpp lib/Frontend/CompilerIn

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

2018-01-05 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 128736. modocache added a comment. Test -Xclang as well. Thanks, @v.g.vassilev! Repository: rC Clang https://reviews.llvm.org/D41733 Files: include/clang/Basic/DiagnosticDriverKinds.td lib/Driver/Driver.cpp lib/Frontend/CompilerInvocation.cpp t

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

2018-01-05 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC321917: [Driver] Suggest correctly spelled driver options (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D41733?vs=128736&id=128824#toc Repository: rC Clang

[PATCH] D41820: [coroutines] Pass coro func args to promise ctor

2018-01-08 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: rsmith, GorNishanov, eric_niebler. Herald added a subscriber: EricWF. Use corutine function arguments to initialize a promise type, but only if the promise type defines a constructor that takes those arguments. Otherwise, fall back to the

[PATCH] D41867: [Frontend] Remove unused FileMgr in pp arg parse

2018-01-09 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added a reviewer: v.g.vassilev. A FIXME added 8 years ago (2010) in https://reviews.llvm.org/rL118203 mentioned that a FileManager should not need to be used when parsing preprocessor arguments. In fact, its only use was removed 6 years ago (2012), in htt

[PATCH] D41867: [Frontend] Remove unused FileMgr in pp arg parse

2018-01-09 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC322118: [Frontend] Remove unused FileMgr in pp arg parse (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D41867?vs=129089&id=129152#toc Repository: rC Clang

[PATCH] D41867: [Frontend] Remove unused FileMgr in pp arg parse

2018-01-09 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Great, thanks for the review, @v.g.vassilev! Repository: rC Clang https://reviews.llvm.org/D41867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41912: [Driver] Test for correct '--version' suggestion

2018-01-10 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: v.g.vassilev, teemperor, ruiu, jroelofs, yamaguchi. The `llvm::OptTable::findNearest` bug fixed in https://reviews.llvm.org/D41873 manifested itself as the following erroneous message when invoking Clang: clang -version clang-6.0: er

[PATCH] D41912: [Driver] Test for correct '--version' suggestion

2018-01-10 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC30: [Driver] Test for correct '--version' suggestion (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D41912?vs=129314&id=129321#toc Repository: rC Clang

[PATCH] D41912: [Driver] Test for correct '--version' suggestion

2018-01-10 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. Thanks, will do next time :) Repository: rC Clang https://reviews.llvm.org/D41912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43927: [Coroutines] Schedule coro-split before asan

2018-02-28 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, lewissbaker, EricWF. The docs for the LLVM coroutines intrinsic `@llvm.coro.id` state that "The second argument, if not null, designates a particular alloca instruction to be a coroutine promise." However, if the address san

[PATCH] D43927: [Coroutines] Schedule coro-split before asan

2018-02-28 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment. I wasn't sure what the best way to test this would be. The assertion occurs in LLVM, but Clang is responsible for scheduling the passes. If anyone has any suggestions, I'd greatly appreciate them! Repository: rC Clang https://reviews.llvm.org/D43927 ___

[PATCH] D44552: [Coroutines] Find custom allocators in class scope

2018-03-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, eric_niebler, lewissbaker. Herald added a subscriber: EricWF. https://reviews.llvm.org/rL325291 implemented Coroutines TS N4723 section [dcl.fct.def.coroutine]/7, but it performed lookup of allocator functions within both the

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

2018-03-19 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, vsk, eric_niebler, lewissbaker. As explained in http://lists.llvm.org/pipermail/llvm-dev/2018-March/121924.html, the LLVM coroutines transforms are not yet able to move the instructions for UBSan null checking past coroutine

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

2018-06-02 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision. modocache added reviewers: GorNishanov, EricWF. Herald added a subscriber: cfe-commits. In his review of https://reviews.llvm.org/D45860, @GorNishanov suggested avoiding generating additional exception-handling IR in the case that the resume function was marked as

[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

  1   2   3   >