[PATCH] D83788: Removed unused variable in clang

modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. No problem, thanks for this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83788/new/ https://reviews.llvm.org/D83788 ___

[PATCH] D83788: Removed unused variable in clang

modocache added a comment. By the way, I tried to accept this diff and leave the following inline comment on `SemaExpr.cpp:15799`: > I guess this assert was never capable of being hit previously, since the > `FixItHint::isNull` would always return true? I wonder if we'll now see some >

[PATCH] D83788: Removed unused variable in clang

modocache added a comment. This LGTM! Looks like the last time this variable was touched was in 2010 as part of a mechanical renaming, https://github.com/llvm/llvm-project/commit/a771f46c82d7. At that point it was used, as a parameter to the static function `MakeObjCStringLiteralFixItHint`.

[PATCH] D82986: [Coroutines] Fix test breakage in D82928

modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82986/new/ https://reviews.llvm.org/D82986

[PATCH] D82928: [Coroutines] Fix code coverage for coroutine

modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Excellent, thanks for this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82928/new/ https://reviews.llvm.org/D82928

[PATCH] D82332: [Coroutines] Handle dependent promise types for final_suspend non-throw check

modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Thanks, this looks good to me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82332/new/ https://reviews.llvm.org/D82332

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Sweet! Thanks for the reviews/responses, LGTM :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82029/new/ https://reviews.llvm.org/D82029

[PATCH] D81885: [Coroutines] Return false on error of buildSuspends

modocache requested changes to this revision. modocache added a comment. This revision now requires changes to proceed. I don't have a preference as to whether `Sema::ActOnCoroutineBodyStart` returns `true` or `false` to indicate failure, although I think returning `true` for failures is a bit

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

modocache added a reviewer: junparser. modocache added a subscriber: junparser. modocache added a comment. Excellent, thank you! The test failures on the diff appear to be legitimate, they reproduce for me when I apply this patch to my local checkout and run `ninja check-clang`. Could you take

[PATCH] D70555: [coroutines] Don't build promise init with no args

This revision was automatically updated to reflect the committed changes. Closed by commit rG627e01feb718: [coroutines] Dont build promise init with no args (authored by modocache). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70555/new/

[PATCH] D70555: [coroutines] Don't build promise init with no args

modocache added a comment. Of course, your approval is very welcome!  I'll go ahead and land this today, thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70555/new/ https://reviews.llvm.org/D70555

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

modocache added a comment. Awesome, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75579/new/ https://reviews.llvm.org/D75579 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

modocache added a comment. I may be wrong, but I believe this breaks `check-llvm` when run with the following configuration: `cmake -DLLVM_BINUTILS_INCDIR=/path/to/binutils/include`: Failing Tests (6): LLVM :: tools/gold/X86/common_thinlto.ll LLVM :: tools/gold/X86/emit-llvm.ll

[PATCH] D75542: [Sema] Prevent UB for uninitialized `IsSurrogate`

modocache abandoned this revision. modocache added a comment. Awesome, thanks! > Alternatively, I considered modifying the `clang::OverloadCandidate` > constructor to initialize `IsSurrogate` with a false value. I feel doing so > would be safer, but I stuck with what appears to be the

[PATCH] D75542: [Sema] Prevent UB for uninitialized `IsSurrogate`

modocache added a comment. Friendly ping! `OverloadCandidate` has uninitialized members and so can cause UB if used incorrectly in another part of the compiler, so I feel this is a fairly straightforward patch: prevent UB by initializing all members. But I'd like some review to confirm.

[PATCH] D75542: [Sema] Prevent UB for uninitialized `IsSurrogate`

modocache marked an inline comment as done. modocache added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:7371 return; } @rsmith I'd definitely appreciate any pointers here -- if initializing `IsSurrogate` with a value as I do in this

[PATCH] D75542: [Sema] Prevent UB for uninitialized `IsSurrogate`

modocache created this revision. modocache added reviewers: rsmith, RKSimon, aaron.ballman, wenlei. Herald added a project: clang. A closed-source C++ codebase I help maintain began hitting a Clang ICE with a stack trace that referenced `clang::OverloadCandidate::getNumParams`:

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

This revision was automatically updated to reflect the committed changes. Closed by commit rG048239e46e49: [Coroutines][6/6] Clang schedules new passes (authored by modocache). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71903/new/

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

modocache added inline comments. Comment at: llvm/include/llvm/IR/IRBuilder.h:137 + + Value *Insert(Value *V, const Twine& = "") const { +if (Instruction *I = dyn_cast(V)) I experienced a tiny regression after applying this patch, and I think this line

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache updated this revision to Diff 245054. modocache removed a reviewer: wenlei. modocache removed subscribers: wenlei, hiraditya. modocache removed a project: LLVM. modocache added a comment. Rebase on top of the latest version of D71902 . Repository: rG

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache updated this revision to Diff 244566. modocache added a comment. Clean up coroutine intrinsics as part of the ThinLTO pre-link pipeline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71903/new/ https://reviews.llvm.org/D71903 Files:

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache updated this revision to Diff 239437. modocache added a comment. Rebase past D72547 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71903/new/ https://reviews.llvm.org/D71903 Files:

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache updated this revision to Diff 236967. modocache added a comment. Initialize PipelineTuningOptions properly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71903/new/ https://reviews.llvm.org/D71903 Files:

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache updated this revision to Diff 236889. modocache added a comment. Update tests -- we now re-run the SCC pass, but don't insert the coroutine funclets into the SCC, so we no longer see the funclets in the output being tested here. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache updated this revision to Diff 236245. modocache removed a subscriber: wenlei. modocache added a comment. Herald added a subscriber: hiraditya. Herald added a project: LLVM. Thanks for the reviews. Based on my latest revision of D71899 , the coro-split

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache added a comment. I'm currently working on ensuring that CGSCC optimizations are rerun to optimize coroutine funclets -- the primary feedback I received on this and on D71899 -- but I just realized I didn't respond to one comment on this set of

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache planned changes to this revision. modocache marked 2 inline comments as done and an inline comment as not done. modocache added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1227 +

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache marked an inline comment as done. modocache added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1227 + MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass())); +

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

modocache created this revision. modocache added reviewers: GorNishanov, lewissbaker, chandlerc, junparser. Herald added subscribers: cfe-commits, EricWF. Herald added a project: clang. Depends on https://reviews.llvm.org/D71902. The last in a series of six patches that ports the LLVM coroutines

[PATCH] D71709: Give frontend dump flags consistent names (*-dump instead of dump-*)

modocache added a subscriber: mehdi_amini. modocache added a comment. Ah, I'm sorry I wasn't clear -- instead of changing a lot of tests to use the new names exclusively, my suggestion was to change one or two tests to use the new canonical name, and have the remaining tests keep using the

[PATCH] D71731: [Format] fix dereference of pointers in co_yeld and co_return statements

modocache added a comment. Nice! Thanks for this. This looks good to me, but I'll defer to the other reviewers you specified, since I think they're more familiar with clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71731/new/

[PATCH] D71709: Give frontend dump flags consistent names (*-dump instead of dump-*)

modocache requested changes to this revision. modocache added subscribers: jroelofs, echristo. modocache added a comment. This revision now requires changes to proceed. Grepping for "dump-tokens", I can see one regression test that exercises this option: clang/test/Lexer/dollar-idents.c.

[PATCH] D71542: [coroutines][PR41909] don't build dependent coroutine statements if the coroutine still has a dependent promise type

modocache requested changes to this revision. modocache added a comment. This revision now requires changes to proceed. Great minds think alike! This looks like the patch I sent in November, D70579 . I only just committed it two days ago in rG376cf43

[PATCH] D70579: [coroutines][PR41909] Generalize fix from D62550

modocache added a comment. Thanks for the review! Much appreciated :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70579/new/ https://reviews.llvm.org/D70579 ___ cfe-commits mailing list

[PATCH] D70579: [coroutines][PR41909] Generalize fix from D62550

This revision was automatically updated to reflect the committed changes. Closed by commit rG376cf43729c8: [coroutines][PR41909] Generalize fix from D62550 (authored by modocache). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70579/new/

[PATCH] D70219: Make `-fmodule-file==` apply to .pcm file compilations

modocache added a comment. This seems like a outright improvement, but as mentioned above it would be nice to get @rsmith's take on it since I'm not an expert. Two nitpicks, though: 1. Could you please run clang-format on the lines you added/modified? 2. Could you apply these changes on top of

[PATCH] D70579: [coroutines][PR41909] Generalize fix from D62550

modocache added a comment. @GorNishanov, @rsmith, friendly ping! @rsmith this patch addresses your requests from https://reviews.llvm.org/D62550. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70579/new/ https://reviews.llvm.org/D70579

[PATCH] D69180: [Format] Add format check for coroutine keywords with negative numbers

modocache added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1761 + tok::kw_case, tok::at, tok::l_brace, tok::kw_throw, + tok::kw_co_return, tok_kw_co_yield)) return TT_UnaryOperator;

[PATCH] D69180: [Format] Add format check for coroutine keywords with negative numbers

This revision was automatically updated to reflect the committed changes. Closed by commit rG8682d29a2877: [Format] Add format check for coroutine keywords with negative numbers (authored by modocache). Changed prior to commit: https://reviews.llvm.org/D69180?vs=225672=231596#toc Repository:

[PATCH] D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves

modocache added a comment. Thanks again for the patch @junparser! And sorry the review took so long! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69022/new/ https://reviews.llvm.org/D69022 ___

[PATCH] D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves

This revision was automatically updated to reflect the committed changes. Closed by commit rG0b3d1d1348da: [coroutines] Remove assert on CoroutineParameterMoves in Sema… (authored by modocache). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69180: [Format] Add format check for coroutine keywords with negative numbers

modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. LGTM! Let me know if you'd like me to commit this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69180/new/

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

modocache abandoned this revision. modocache added a comment. I'm not super interested in this patch anymore, someone else feel free to work on this! :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62035/new/ https://reviews.llvm.org/D62035

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

modocache abandoned this revision. modocache added a comment. I'm not super interested in this patch anymore, someone else feel free to work on this! :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59765/new/ https://reviews.llvm.org/D59765

[PATCH] D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves

modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. LGTM, thanks! Please let me know if you'd like me to commit this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69022/new/ https://reviews.llvm.org/D69022

[PATCH] D70579: [coroutines][PR41909] Generalize fix from D62550

modocache created this revision. modocache added reviewers: GorNishanov, rsmith, lewissbaker. Herald added a subscriber: EricWF. Herald added a project: clang. In https://reviews.llvm.org/D62550 @rsmith pointed out that there are many situations in which a coroutine body statement may be

[PATCH] D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves

modocache requested changes to this revision. modocache added a comment. This revision now requires changes to proceed. Sorry for the slow response here, @junparser! The test case you came up with here is great! I can see the issue is that `ScopeInfo->CoroutineParameterMoves` are built up when

[PATCH] D70555: [coroutines] Don't build promise init with no args

modocache created this revision. modocache added reviewers: GorNishanov, rsmith, lewissbaker. Herald added subscribers: cfe-commits, EricWF. Herald added a project: clang. modocache edited the summary of this revision. In the case of a coroutine that takes no arguments,

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

modocache updated this revision to Diff 227808. modocache added a comment. Rebasing onto the monorepo. @rsmith, I confirmed the test cases that this diff adds still fail on trunk, and that the Clang source changes made in this diff fix the test case failures. Tomorrow I plan on poking around to

[PATCH] D69144: [Format] Add format check for throwing negative numbers

This revision was automatically updated to reflect the committed changes. Closed by commit rG7f704320b058: [Format] Add format check for throwing negative numbers (authored by modocache). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69144/new/

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

modocache added a subscriber: lewissbaker. modocache added a comment. > Is that maybe intentional, and is the code not intended to compile? It looks like it should work to me, but maybe @lewissbaker or @GorNishanov can answer definitively. Comment at:

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

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] D65043: [Format] Add C++20 standard to style options

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] D44672: [CodeGen] Disable UBSan for coroutine functions

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

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

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:

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

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

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

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

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

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

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

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

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] D65149: [Format] Add test demonstrating PR42722

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

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

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

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] D65183: [Format] Make it easy to add new format::FormatStyle::LanguageStandard. NFCI

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) &&

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

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] D65149: [Format] Add test demonstrating PR42722

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

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] D65043: [Format] Add C++20 standard to style options

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] D62550: [coroutines][PR41909] Don't build dependent coroutine statements for generic lambda

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

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

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL362348: [coroutines][PR41909] Dont build dependent coroutine statements for generic… (authored by modocache, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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] D59765: [Lex] Warn about invisible Hangul whitespace

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] D59765: [Lex] Warn about invisible Hangul whitespace

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] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

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

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

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=192050#toc

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

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

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

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

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

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

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

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

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

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

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

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

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=190890#toc

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

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

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

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] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

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

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

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

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

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,

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

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:

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

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

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

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) <<

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

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

  1   2   3   >