- User Since
- May 27 2014, 6:39 AM (234 w, 1 d)
Fri, Nov 16
Have you considered the same approach as typo correction? I.e. for the max allowed edit distance use percentage of the input size. For example, something similar to TypoCorrectionConsumer::addName.
Thu, Nov 15
Tue, Nov 13
The test case I've promised is
Fri, Nov 9
Thu, Nov 8
How do you want it to work with symlinks? Would getRealPath be sufficient for your purpose?
Wed, Nov 7
Depending on isLocal usage, it can be worth implementing it for RedirectingFileSystem.
Tue, Nov 6
OK. Good to know you are still working on it.
Mon, Nov 5
Sorry, you've decided to abandon the patch, it took a lot of good work. Xing, are you sure you don't want to see this change finished? I agree that delays in code review can be frustrating and I think it is something we can improve.
Disabling test in D54132.
Thu, Nov 1
Thanks for the review, John. I'll update the comment with TODO and commit.
Wed, Oct 31
Can you please check https://reviews.llvm.org/D53674 ? It builds on top of this change and I plan to commit them together.
Exclude commits tracked in a different review.
- Rename EmitConstant to emitScalarConstant.
- Tweak comment to be explicitly about intended IR code, not about Obj-C++ code.
- Switch to camelCase.
Looks good to me.
Tue, Oct 30
- Rename EmitConstant to EmitScalarConstant.
Mon, Oct 29
I am gravitating towards having a separate unit test(s) for no_push functionality. Maybe I'm wrong but I expect it to be smaller and easier to understand, though there'll be some boilerplate.
Fri, Oct 26
Thu, Oct 25
- Rebase once again.
- Use Doxygen comments in some places.
- Address review comments.
Wed, Oct 24
Thanks for the review.
Tue, Oct 23
Have a few comments but didn't try to come up with edge cases yet.
Oct 16 2018
Sorry about the delay. The change seems to be correct but ninja check-clang reveals the test "Misc/caret-diags-macros.c" is failing. Can you please look into that?
- Rebase on top of the latest changes in trunk.
- Address review comments.
Oct 12 2018
Sep 21 2018
It seems like there are too many asserts and they are too specific, they seem to be aimed at specific potential bugs. What about asserts that make sure we maintain some invariants? For example, check DiagStr < DiagEnd once in a loop instead of every place we increment DiagStr. Do you think it should catch the same problems but maybe a little bit later?
Sep 20 2018
Thanks for review.
Sep 19 2018
The repro is
Reverted in r342599.
Seems like revert didn't go through. I suspect it is causing build failure in http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/49593/consoleText
I have published D52278 for disabling the test on Darwin. So far my investigation shows that __xray_log_select_mode("xray-fdr") returns 2 (aka XRAY_MODE_NOT_FOUND) and that's why we are calling std::abort.
Sep 18 2018
Thanks for the fix.
Confirm that reverting the change locally fixes the tests. If nobody beats me to it, I plan to revert the change in 30-60 minutes. @srhines, if you want to fix it in another way and need more time, please let me know.
Seems like this change causes 2 test failures:
Sep 17 2018
I've reduced the input causing error to
Who is responsible for updating the clients? Currently in the test suite MicroBenchmarks/XRay/FDRMode/fdrmode-bench.cc is failing to compile because it is using __xray::FDRLoggingOptions.
@dexonsmith, does my change address your concerns?
Looks like this changed causes errors in backend in stage 2 on Apple platforms
Sep 14 2018
- Improve tests, fix -MMD, address comments.
Taking over the change, I'll address the reviewers' comments.
Sep 13 2018
Sep 10 2018
Agree that fatal/non-fatal error is too coarse and tooling/IDEs need more details and more control to provide better experience. But I don't think we are in a state to claim that all errors are recoverable (in theory and in current implementation). Instead of continuing on all errors, I prefer to select errors that are important for tooling and improve those first.
Rebased on top of trunk and checked that this is still working. Please take a look.
Draft is abandoned in favor of D50539.
Regarding the asserts to catch potential problems, seems most of them are for buffer overflows. Aren't sanitizers catching those cases, specifically Address Sanitizer? I haven't checked, just seems it would be good to check buffer overflow automatically instead of using explicit asserts.