- User Since
- Feb 16 2014, 1:10 AM (265 w, 2 d)
Friendly ping, @sylvestre.ledru! I'm also experiencing this build error. It looks like other diffs have been submitted to fix this, such as https://reviews.llvm.org/D59162, but it was abandoned in favor of this patch. It'd be great if you could commit this one.
Fri, Mar 15
Great, thanks for the reviews, everyone!
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 whether this sort of behavior is expected, or if there are known workarounds. Any and all comments appreciated!
Thu, Mar 14
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.
Remove unused function parameter.
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.
Mon, Mar 11
Thanks @GorNishanov! I moved the call into splitCoroutine. I'll land this now.
Oops! Thanks for pointing that out @davide -- I had meant to pass them through to the DeleteDeadBlocks but forgot. Should be good now!
Fri, Mar 8
Friendly ping! I thought this might be easier to review if I split it up from https://reviews.llvm.org/D59068, which fixes a bug in coroutines and has already been accepted. This diff is an NFC but is necessary to land the bug fix.
Thu, Mar 7
Thanks for the review!
Add test case for executing a lambda using co_yield within a catch block.
Wed, Mar 6
Mon, Mar 4
Looks good, thanks for adding a driver option for this!
Fri, Mar 1
Jan 31 2019
Thanks for the reviews!
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!
Jan 30 2019
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::getUnqualifiedType. I modified my patch to use ASTContext::hasSameUnqualifiedType instead.
Jan 21 2019
Jan 10 2019
Great, thanks for the review!
Jan 9 2019
Remove obsoleted code I accidentally included.
Thanks for the offline review @GorNishanov! This revision allows constexpr usages of __builtin_coro_frame_max_size.
Jan 6 2019
Thank you for the review!
Jan 3 2019
Jan 2 2019
Dec 21 2018
Excellent progress! Thanks for working on this.
Dec 19 2018
Dec 18 2018
Dec 17 2018
Dec 12 2018
Dec 11 2018
LGTM! As it happens I was reading this code the other day and wondering why it wasn't able to eliminate a suspend point I'd been looking at. I'll try it again and see if this new behavior is able to eliminate -- thanks!
Dec 10 2018
Sorry for the wait! Mostly nits, but also I think the while condition might be inverted? It seems like, as is, it would never execute the body of the loop...?
Dec 9 2018
Thanks for the review, @reames! I implemented each of your suggestions, but I had a small excuse for not using the variable name Suffix.
I see, thanks @arsenm! I think this variable fits those criteria so I changed it back to using a dollar-sign prefix. Variable names notwithstanding, does the implementation change seem OK to everyone? It fixes what I believe is clearly a programming error. Landing this change will unblock me from LLVM_NODISCARD to these functions and so preventing another case of this same error.
Search the bugpoint directory first, and only fall back to PATH if the program isn't there.
Thanks @MatzeB! I'll change this to search in the bugpoint directory first, and fall back to the PATH.
Thanks for the review, @nhaehnle! I changed the FileCheck variable NOUNWIND_READONLY to not use a dollar-sign prefix. I'm not sure why the other lines in this test do, but I left them alone for now. Any other suggestions for this, or is it good to go?
Dec 7 2018
Dec 5 2018
Oops, sorry, I didn't check hard enough. There's code for AMDGPU that runs up against this warning, causing some buildbots to fail: http://lab.llvm.org:8011/builders/lld-perf-testsuite/builds/9637/steps/build-bin%2Flld/logs/stdio
Thanks! Yup, I confirmed no new warnings when building check-llvm and check-clang.
Dec 3 2018
Nov 28 2018
Friendly ping! Could someone recommend a reviewer for this? Or is there something wrong with the patch?
Friendly ping! I like this patch because it saves me the keystrokes of typing --opt-command every time I run bugpoint. Please give it a look and let me know if I can improve anything.
Nov 25 2018
Nov 5 2018
Sorry to have let this languish! LGTM.
Nov 3 2018
Oct 30 2018
Oct 29 2018
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_
Oct 15 2018
Oh, I'm sorry I let this languish! I'll address your comments later this week, @vsk. Thanks so much for the review!