User Details
- User Since
- Oct 18 2019, 11:34 AM (178 w, 6 d)
Thu, Feb 23
Feb 21 2023
Feb 17 2023
Are you good with this now @serge-sans-paille ?
Feb 15 2023
@jrtc27 raises a good point - we have discussed deprecating the option but made it a hard error. Should we instead make it a soft error for now and a hard error later on?
Feb 14 2023
Thanks for the review - removed those lists(). Wonder why 2to3 is so aggressive with inserting them? anyway - it seems to parse fine afterwards. but since I don't know the files this should be executed on it's hard for me to test the functionallity.
Removed unecessary lists
Feb 13 2023
My motivation was that people asked to add support for new cache programs (sscache and more). And now that this is supported in upstream CMake in a nice way I think our build system is way too big and complicated to duplicate functionality that's already in upstream CMake. It's more switches to keep testing and update.
Thanks!
Thanks for the review and the explanations! I fixed your nits and landed this.
Thanks for your comments - feel free to comment on the release note, I was struggling with describing the fix well for a short release note paragraph.
- Expand on tests
- Fix crash when Attrs was null
- Added release note
Thanks!
Feb 12 2023
Basically every bot broke with this change, so I reverted it for now and posted to discourse: https://discourse.llvm.org/t/llvm-ccache-build-is-deprecated/68431
Ping.
Feb 10 2023
Thanks for your review @kwk - I'll merge this now and start working on the action trigger.
Expanded and fixed comments
Feb 9 2023
Fix comment
Thanks for your suggestions. I have pushed a new diff with the fixes.
Incorporated feedback
clearing my queue
Hi - I finally got around to picking this one up again. This is rebased, passing all tests and now we pass the std::error_code directly to the API and changed all the callers to itaniumDemangle() and changed most of the places to actually check the error_code instead of the return const char*.
rebased and implemented some of the suggestions.
@brad is this something you still need for OpenBSD?
Clear my queue.
Feb 8 2023
Thanks!
Feb 7 2023
- Generate remote and branch names
- Make terminology a lot clearer
- Make cleanup optional
Thanks for working on this! I have a bunch of comments :)
Added warning when the iOS sdk is not found.
Reworded the commit message to reflect the current state of the patch
Add a warning when you try to pass LLVM_CCACHE_BUILD=ON to CMake
Python is already a hard requirement for many things in the LLVM ecosystem, so I don't think that is an argument. Other than that I don't know enough about Bazel to know if this is good or not.
Happy to do a deprecation warning or error.
Feb 6 2023
We could add deprecation warning for this something like:
I fixed the obvious errors and re-testing it and it seems to behave correctly now. Sorry for the noise.
Addressed comments
I have been using "black" to do python formatting in other places. Maybe we should make that part of the LLVM dev guide. But I think it's better if we make formatting changes separately from the functional changes.
Whoops. Bad copy and paste job. I first made it into a macro and did it for all three SDKs and then realised watchos and tvos is off by default and don't need that probing. Then I probably messed it up when removing the macro. Pretty sure I tested it in this form though ... hmm. Anyway I agree with your comments and will address and retest it tomorrow.
A long time later, I finally got time to test this since it was marked as necessary to fix before 16.x release.
Addressed comments
Feb 2 2023
I think this is fine. This warning is not very useful when used in this way.
Feb 1 2023
Seems fine to me. Have you tried clang-format locally since the CI job seems broken?
Jan 27 2023
we really should centralize the C++ standard variable in top-level cmake or llvm.
I thought the required version is much less than 3.10? Why is this helping / failing?
Took me a while to see the difference between the new and old line. Good spot.
Jan 17 2023
I haven't tried the change - but the errors here makes sense. I bet those objects have references to the other targets and they will differ when we disable anything but Native. If we want to keep these checks we probably need to build all targets for phase 2 or filter the list of objects to compare.
Makes sense to me. We probably should add bolt in there as well since it's enabled by default now, cc @Amir
Jan 13 2023
Good change!
Good plan. Let's merge this and we can do another with lld later.
Jan 12 2023
I think we should include lld as well - now that LLD can produce production binaries on all platforms I think it makes sense to prefer that over system linker.
Jan 8 2023
Thanks for making this change. I think the changes to the bump script looks fine if no processing is needed in the future.
Jan 2 2023
Looks like it's ready to go to me. Maybe take a look at the comments inline in the review and mark them as done or reply to them, there were some questions in there that I couldn't see any replies to.
Dec 29 2022
Dec 21 2022
Oh sorry for the late reply. I think removing the getter makes sense as well - it worked better when we had the earlier revision that copied string out of the Error etc.
Dec 19 2022
Dec 9 2022
It still looks odd to me, but since the rest of the file has comments like that I am fine with it. But I think we should figure out if they are like that because some tool is parsing them or if it's just legacy.
Dec 8 2022
Dec 6 2022
If this fixes OpenBSD it looks fine I think. But I wonder if we shouldn't just do this if we are on OpenBSD, changing the SOVERSION has been fraught with problems before since people have scripts that expect certain layouts.
Is there any point in adding any tests here that can be useful? Probably not since we are working with the Windows API right? I don't have much to comment on, but it seems reasonable if it solves the problem.
I think this is ready to land @Mordante or is there anything else missing?