- User Since
- Jan 10 2013, 2:43 PM (253 w, 4 d)
Tue, Oct 31
It could be justerrorToBool(BCData.takeError());
and errorToBool() can handle ErrorSuccess, no?
I'm not subscribed to llvm-commits so replying here:
Now with better test. I'm going to land this; happy to address post-commit review comments of course.
Now with test (only for one of the 4 changes, but the one we hit in practice. I don't see how to test the other 3). Feeling pretty good about this now.
Mon, Oct 30
Sounds like this ended up being useful for Android too: https://github.com/android-ndk/ndk/issues/105#issuecomment-324179958
Thu, Oct 26
Wed, Oct 25
In general, "I don't see any way to prevent $slow_thing" just means the patch can't move ahead. But in this case, this looks like fairly small overhead for a worst-case input, so this won't be slow on real-world code. So lgtm.
Awesome, thanks much! Like lebedev.ri says, adding a test for the "Parens" part would be good.
Can you build some large-ish codebase (say, LLVM) with and without this patch and make sure that this doesn't measurably add to build perf? (With the warning turned on, obviously.)
Mon, Oct 23
Oct 20 2017
because this requires Windows an
Yes, that's what I meant. clang-cl also has a bunch of Unix style flags. I agree it's a bit strange, but it'd be llvm-consistent.
Oct 17 2017
Oct 16 2017
This would work for me, but it sounds like it'd break mstorsjo :-( Maybe lld-link and clang-cl should both just have a --version. That won't clash, does what it says, and won't break existing code.
As said on the bug, this matches gcc's behavior, and with this you won't see this warning for NULL. I don't think this is better.
Oct 6 2017
Should probably get a release note too.
Sep 27 2017
Sep 26 2017
Sep 19 2017
I think for depfile generation, we generally try to be gcc-compatible (I can link to prior changes in this spirit), so I think this seems like a good thing to do to me. gcc only does this for system headers, yes?
Sep 5 2017
Sep 4 2017
fix a comment typo
Actually, I talked myself out of this affecting %LINK% / %_LINK_% while writing the patch summary. I removed that bit.
Aug 30 2017
Thanks, landed in r312167.
Aug 28 2017
Don't warn in unevaluated contexts. Ready for a look now.
Aug 23 2017
Aug 22 2017
add test from rnk
Aug 21 2017
Just use TryTerminatedBlock
Aug 18 2017
Many driver tests check in a basic representative directory structure (e.g. test/Driver/Inputs/basic_freebsd_tree/ and its many siblings).
This approach looks good to me.
Oh, I guess this is superseded by https://reviews.llvm.org/D36860 ?
Aug 16 2017
Aug 14 2017
Aug 9 2017
Aug 8 2017
Aug 7 2017
Aug 4 2017
Why should this be part of llvm? This seems to come with very heavy dependencies (protobuf), and LLVM has historically tried to minimize the number of things it depends on.
Is landing this blocked on internal testing?
Hm, that's unfortunate. I don't see a good way to rescue this patch.
Aug 3 2017
I'm going to land this. It's early in the 6.0 cycle, so if this causes issues, we should have time to find them and then follow up in case we run into any.
I've reverted this in 309960, as discussed.
(Our workaround is to call __builtin_available() once before engaging the sandbox, which isn't so bad. Just thought I'd let you know about it; this isn't a serious bug for us.)
Looks like the email reply didn't make it to phab, so here it is again:
Aug 2 2017
We just noticed that if you call __builtin_available() for the first time after activating your app's sandbox, the function will fail:
Awesome, thanks! Maybe mention the PR# for this in the commit message.
Jul 28 2017
dim: Does putting the target listing behind a different flag work for you? Which problem are you trying to solve here?
Jul 26 2017
Jul 25 2017
Jul 24 2017
Sorry, I just noticed this weeks later. Why are we adding this to --version instead of adding some new flag for printing this? When I pass --version, I'm usually interested in clang's version and don't need a screenful of other information below it (which makes the output I do care about scroll off the screen).
Jul 23 2017
Most of the patch is unifying all the toolchains to call the newly-introduced ToolChain::ShouldLinkCXXStdlib() instead of all manually checking for D.CCIsCXX() && !getFlag(nostdlib, nodefaultlibs). The actual behavior change is to make that function check the new nostdlib++ flag too.
Jul 21 2017
From what I understand, _MSC_VER changes with each 2017 update.
Jul 17 2017
Jul 14 2017
I went ahead and landed this in r308044, given that I addressed the nits and removed the possibly contentious bits. Happy to address remaining nits in a follow-up.
Mostly done, thanks!
Thanks, all done, much better!
Jul 13 2017
Document *, add example with multiple platforms
Jun 27 2017