- User Since
- Jan 10 2013, 2:43 PM (267 w, 2 d)
Thu, Feb 15
So should we revert https://reviews.llvm.org/D42632 for now?
Did this land?
Tue, Feb 13
Mon, Feb 12
some attr merging
Fri, Feb 9
Thu, Feb 8
I was about to say "please make sure this honors -fdebug-prefix-map" but I suppose codeview probably contains so many absolute paths at the moment that we should instead have a concerted effort to make debug info cwd-independent at some later point instead.
Wed, Feb 7
Err sorry, landed in rL310382.
What's the status here?
Tue, Feb 6
I went ahead and landed this with the comments requested by Duncan in r324385. (http://llvm.org/viewvc/llvm-project?view=revision&revision=324385)
Mon, Feb 5
This should be in clang-tools-extra next to clang-tidy, clang-include-fixer, clangd etc, not in the main compiler repo, right?
Thu, Feb 1
The powers that be updated the SDK on the chromium clang bots, so we need some solution for this issue soon.
Jan 25 2018
Jan 24 2018
Jan 23 2018
(Rafael reviewed at http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180122/518553.html , so closing in phab)
(addressing comments at https://reviews.llvm.org/D42422)
Jan 17 2018
Jan 16 2018
Yes, I looked at the same file and came to the same conclusion regarding a test :-)
Jan 15 2018
Jan 2 2018
FWIW we build with -Wall -Wextra and we disable this warning since it's in our (chromium's) opinion not useful on large real-world code. So I'm not sure it should be in -Wextra. (Also, I believe clang has historically tried to keep -Wall and -Wextra pretty similar?)
Dec 28 2017
(to keep things linked up, this broke building chromium, fixed by inglorion in r321512)
Oct 31 2017
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.
Oct 30 2017
Sounds like this ended up being useful for Android too: https://github.com/android-ndk/ndk/issues/105#issuecomment-324179958
Oct 26 2017
Oct 25 2017
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.)
Oct 23 2017
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.