- User Since
- Feb 11 2015, 3:26 PM (257 w, 4 d)
Tue, Jan 14
Dec 12 2019
Dec 10 2019
Thanks a lot for your work (and anyone else that's participated). I've left a few comments, but obviously this patch is huge. @STL_MSFT How do you want to proceed with this? We can either back-and-forth on this review until we're happy and we can ship it (my preference), or if you prefer someone from libc++ can take over the patch from here. My preference is clearly the former, but I want to avoid annoying you in case you don't have time for this.
Is import distro supported in Python 2.x?
Nov 18 2019
It would be great to document these macros somewhere in the code or in a .rst document.
I love this. It's more verbose but it also gives us more explicit control over our ABI list, which is great. We'll run into problems when wanting to explicitly instantiate the vtable of types that have one (if we eventually generalize this approach to other types). This is one of the reasons why I wrote http://wg21.link/p1263. However, none of this is needed for just std::string.
Nov 14 2019
Except for the nitpick, LGTM.
Nov 13 2019
Sorry if this is a basic question, but why are we trying to provide EINTEGRITY in libc++'s errno.h header at all? I don't see this being part of the C or C++ Standard.
Nov 11 2019
Nov 7 2019
https://reviews.llvm.org/D69940 should fix the problems.
Once you guys have figured out how you want to proceed, please say it here and abandon one of the two revisions. I'll review the one you decide to keep.
Folks, we're aware of the CI failures, and we're working on fixing them. It's taking a bit longer than usual since we're at a committee meeting right now.
In addition, I'd like us to remove support for the out-of-tree build (I'll make a separate PR for that).
Nov 6 2019
Tested locally and it gives the same results as the previous script.
When running the tests locally, I see errors that are probably a result of messing around with the environment:
This looks good to me, but I must admit I'm surprised this problem has never come up before (this code is very old), and also I don't know what the code is trying to do. Adding Saleem and Nick who have more experience with libunwind just to double-check.
Nov 5 2019
This looks fine to me, but like I said I don't run the tests on Windows so I can't test locally. Since no one else seems objected, I'll commit this and folks can chime in if they're unhappy. Otherwise we're just blocking you for no good reason.
Nov 4 2019
Woops, it also looks like you are missing the feature test macros:
Oct 30 2019
Oct 29 2019
Oct 25 2019
Oct 24 2019
LGTM with the requested change.
Oct 23 2019
Oct 22 2019
Some of the tests you added are already present in other files, for example libcxx/test/std/input.output/iostreams.base/fpos/fpos.operations/subtraction.pass.cpp. Any reason why you're duplicating those checks in test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp?
Good find! The following drivers (at least) appear to have the same problem:
Oct 21 2019
Oct 18 2019
Requesting changes so it shows up properly in my review queue.
Oct 16 2019
If we want to mark the LWG issue as being resolved for libc++, I think the test should be moved to the libc++ test suite. Otherwise, we're only assessing that libc++ implements LWG2426 on Clang.
Let's not block this because I have questions about the warning implemented in Clang. Richard, you can ship this. Thanks for the fix and sorry for the delay.
Confirmed, this has been applied in r373971 so I'm closing this.
Sorry for the spam, I'm trying to close this and it won't let me. Now I'm trying to remove Eric (who had requested changes).
Closing since the patch has been applied. We couldn't close it before because there were pending changes requested by Eric.
Commandeering revision to try to close it (I'm doing some cleanup in the review queue).
Can you confirm this is the bit of the patch that we suspect had caused a SEGFAULT for @phosek?
I think this has been committed (r373971)? If so, can you please close?
Looks good, except for the duplicated test() functions. I'd also like to have a short discussion about how to test this (and more importantly the lack of regression w.r.t. codegen). Apart from that, I think this is good to go.
This is looking good. I especially like how the tests require only minimal change between the runtime and the compile-time version. I think this is what we should strive for pretty much every time we constexpr-ify something going forward.
Oct 15 2019
Note: There does not appear to be anything at https://github.com/llvm/llvm-project/compare/master...martijnvels:string-optimize-copy-ctor
Oct 11 2019
@phosek Can you please confirm this also fixes your issue in the Runtimes build?