- User Since
- Aug 29 2012, 11:21 AM (363 w, 6 d)
Fri, Aug 16
Committed as rL369129.
libcxx is using llvm-config to find the CMake exports; that's actually what prompted this change.
Thu, Aug 15
Thu, Jul 25
I'm personally still of the opinion that allowing non-trivial fields in unions was a mistake, but it's too late to change that as well.
Wed, Jul 24
Sure, I guess. I suspect most crashes-while-crashing are from the PrettyStackTrace machinery, not these allocations, but you're right that we can get a partial string out of it if it's short enough.
Oops. It looks like there's another place where this pattern shows up (see rC139382). The other one should probably be changed as well.
Jul 18 2019
Committed as rL366486.
Jul 17 2019
Jul 12 2019
Merged in rL365911.
Jul 11 2019
Jul 1 2019
Yeah, I'll write one.
Jun 28 2019
Jun 25 2019
Made it a per-thread opt-in, leaving the potential for it to be useful in multi-threaded programs. (It's off by default.) Also added a SaveAndRestore of errno for SIGINFO handlers, though this one doesn't need it.
Jun 24 2019
Jun 17 2019
Apr 17 2019
Mar 29 2019
Ooh, I should have remembered "designated initializer". I guess it doesn't matter so much either way.
I don't think there's ever a reason to call [super self], and doing so through a macro could easily indicate a bug.
Mar 26 2019
Mar 25 2019
This definitely makes sense for Swift's downstream fork of LLVM, but I'm not sure it makes sense upstream. I'm not sure how much special treatment we want to give Swift-the-project in the LLVM repo.
Mar 12 2019
Mar 11 2019
This commit by itself doesn't change any behavior, right?
I'm no Clang serialization expert but this makes sense to me. It's consistent with all the other statement visitor methods.
Jan 23 2019
I think this is reasonable but Doug was the one who worked on this. I wonder if it also helps with the test cases in rdar://problem/24619481.
Dec 21 2018
I think Aaron and John have covered any comments I would make! I'm glad this came out so simply, though. I was afraid it'd be a much larger change than just expanding what was already there.
Aug 30 2018
> If we do take this answer, we should *still* go to all clients and see if a zero-length check makes sense. "Copying" an empty string or empty array should definitely not result in an allocation.
In that case, should we just assert about it in the allocator?
Aug 28 2018
If we do take this answer, we should *still* go to all clients and see if a zero-length check makes sense. "Copying" an empty string or empty array should definitely not result in an allocation.
Aug 27 2018
I'm not sure whether it's better to do this or to remove LLVM_ATTRIBUTE_RETURNS_NONNULL. I'll defer to Hans or others on that, since I'm not a frequent LLVM contributor these days.
Aug 14 2018
I'd like to help but I'm not a full-time LLVM contributor anymore, so I don't think I can be the one to sign off on it. I don't know who works on LLVM's CMake these days.
Jul 23 2018
I'm not much of an LLVM committer these days but I see no problems with this.
Jul 18 2018
Cool stuff! I didn't look at things line-by-line, but here's a few comments anyway.
Jul 16 2018
Jul 10 2018
Sorry for the delay. I think this is mostly good, but I do still have a concern about the diagnostics change.
Jul 3 2018
Jun 28 2018
Jun 18 2018
Thanks, Roman. Too bad this is such a stable piece of LLVM, or else I'd have more people qualified to review!
Jun 15 2018
Committed in rL334841.
Jun 14 2018
Jun 13 2018
Took recommendation about the form of the check, pumped up the tests.
Landed in rL334632. Turns out the iOS.cmake part has been refactored since the version I was looking at (Swift 4.2's branch of LLVM), and so this was just the libtool change.
Reviving this old review. I'd like *someone* to tell me I didn't miss something.
Jun 12 2018
May 7 2018
Landed in rL328345.
Apr 9 2018
Exactly that's what I thought too, and llvm::sort already accepts a range (Container.begin(), Container.end()). In that case, could you please clarify what exactly do you mean by a "range-based variant" and its use cases in LLVM?
Are there instances in llvm where we perform range-based sorting? I see that std has an experimental range-based sort (std::experimental::ranges::sort) which I don't see being used in llvm.
Apr 6 2018
Ah, yes, Graydon's convinced me that the change will work and will not regress memory usage terribly, and the new tests look good.
Apr 2 2018
I'm a little disappointed that llvm::sort got added without providing a range-based variant, the way we did for llvm::none_of and llvm::find and others. (I wasn't watching the original thread.) I see no problem with these changes, though!
Mar 30 2018
That seems about as clever as possible—anything more and it would definitely be overboard. Can you add tests with 255-, 256-, and 257-byte buffers, then? With and without newlines as the last character, and testing the just-past-the-end pointer in addition to something in-bounds?
Mar 29 2018
And those that do, assume a file is (say) 1000 lines long, it'll cost 8kb to index. Even if we indexed a thousand such files -- a diagnostic in every file of a large project! -- we'd only be talking 8mb.
That's a source compatibility break, but not one that's likely to matter in practice. LGTM.
Mar 28 2018
That's potentially a lot more memory usage, since the various vectors never get freed. Have you measured what the effect on memory usage is on a large Swift project with diagnostics in many files?
Mar 22 2018
Committed in rL328276.
Mar 14 2018
Mar 9 2018
Mar 1 2018
It's been a long time since that commit, but I think the difference is that -pedantic / Extension-type diagnostics are magic and -Wc++98-compat-pedantic is not. I like Richard's way better and if it could be applied to -Wnewline-eof later as well then that sounds great.
Feb 26 2018
Go for it!
Feb 20 2018
Feb 19 2018
Nov 27 2017
Oct 23 2017
Landed in rL316408.
Oct 20 2017
Oct 17 2017
Oct 13 2017
Committed in rL315697.
Oct 12 2017
Seems reasonable to me. I don't know anything about the MIR parser's use of diagnostics, though.
Oct 11 2017
Sep 27 2017
Nice catch, Volodymyr! Looks good to me.
Sep 25 2017
Remove debugging prints.
@ddunbar convinced me in person that this is either overkill or doesn't go far enough: as long as there's a shared directory, either there can just be a common setup task at the start of each test that does its own ad hoc filesystem-based synchronization, or we might as well go all the way to full-on inter-test dependencies. I'm going to split out the shared directory stuff into a separate patch and go play with the idea of dependencies for a bit.
Sep 19 2017
Move setup code from execute_tests_in_pool to execute_tests.
Yep, that's not bad either. I'm not super happy with the idea of lit.local.cfg parsing relying on running in the same process as lit itself, but maybe that ship has sailed.
Oh, no, I definitely need an arbitrarily complex script. Swift wants to prebuild mock libraries shared by several test cases. Today we're either doing it once per test case or using a terrible hack mode that can parse source on the fly.
You mentioned you want to make this work with the multiprocessing pool. That worries me a little bit because different processes are going to be using the same shared output dir (which isn't created up front by the infrastructure), if I understand correctly. Like if you have A and A/B and A/C, then we will run the setup script 3 times in parallel all using the same shared output folder. Is this going to be a source of flakiness?
Previously I was just able to do this:
Make setup_script refer to a Python script rather than an arbitrary executable, as suggested by @zturner.
I'm unhappy that I can't run lit's own tests without having to configure, though. That seems like a regression.