- User Since
- Aug 29 2012, 11:21 AM (372 w, 5 d)
Fri, Oct 18
Thu, Oct 17
Thu, Oct 10
Committed in rCTE374494.
Committed in rL374440. I split the difference and put the EBO comment in the commit message.
Tue, Oct 8
Committed in rL374102.
Mon, Oct 7
Fixed bad uses of StringSet, changed a friend from StringMapEntry to StringMapEntryStorage. The fact that I only had to do this in one place (and that one place is definitely doing something tricky) makes me still feel confident enough to make this change.
Hm, doesn't quite work yet but I'll get there.
Committed in rL373935 (with the update for AMDGPURegisterBankInfo.cpp).
Fri, Oct 4
Committed as rC373769.
Okay, having Xcode force-load the static libraries doesn't seem bad at all.
Thu, Oct 3
Okay, new patch set at D68439.
I'm not quite sure what it's doing. The executable targets end up trying to link against the static libraries anyway, which of course haven't been built. It's possible that this is because the LIBTYPE is both STATIC and OBJECT and if it were just OBJECT we might be better off, but I'm not sure if Xcode's IDE features will be happy with a target that doesn't actually produce a library. I can try it if you want, though.
Wed, Oct 2
I don't. I know Swift's (hopefully superfluous) version of the same code works fine on the MSVC's we support, though: https://github.com/apple/swift/blob/da1002025323cc4199439b10d4c7bac11322d22c/include/swift/Basic/STLExtras.h#L267
I'd like to try adding this back. Do you remember what the failure was? Maybe we can try calling begin() and end() instead of using the fields directly?
Wed, Sep 25
Definitely in favor. :-)
Sep 3 2019
Aug 16 2019
Committed as rL369129.
libcxx is using llvm-config to find the CMake exports; that's actually what prompted this change.
Aug 15 2019
Jul 25 2019
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.
Jul 24 2019
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.