- User Since
- Jun 25 2014, 4:17 PM (395 w, 1 d)
Fri, Jan 14
Tue, Jan 11
LGTM with one comment. This is great, thanks!
Mon, Jan 10
Wed, Jan 5
Tue, Jan 4
Thanks! I think your tests don't actually test the added code, can you check if you can parse a mapping like ios_tvos in the constructed SDK settings?
I think we would still want to diagnose any mismatches between class properties that override a class property in the superclass. do you think that would be useful? You can adjust the lookup to take that into account.
Dec 20 2021
This change caused the following regression on the EXPENSIVE_CHECKS=on green dragon CI, where clang is unable to compile compiler-rt for armv7:
@vitalybuka Hi, this change caused some test failures on green dragon apple CI. This is the bot: https://green.lab.llvm.org/green/job/clang-stage1-RA/. The following tests are failing:
@vitalybuka This change caused a regression on Darwin CI for tsan -,
This is the CI job: https://green.lab.llvm.org/green/job/clang-stage1-RA, and the build: https://green.lab.llvm.org/green/job/clang-stage1-RA/26354/
ThreadSanitizer-x86_64.Darwin.dlopen.cpp is now failing because the initializer that you added runs before flags is initialized when it's being dlopened . Could you take a look at this issue?
Dec 8 2021
Dec 7 2021
Dec 6 2021
This looks good to me! I agree, an enum class would be cleaner though.
Dec 2 2021
Nov 29 2021
Sorry for delay, this LGTM. Thanks for doing this cleanup!
Nov 19 2021
I will commit this on behalf of @beccadax as she doesn't have commit access yet.
Nov 5 2021
Oct 29 2021
Oct 28 2021
Looks good to me. Can you post the patch with the full context next time so that it's easier to review?
Oct 20 2021
Sep 20 2021
Sep 7 2021
Sep 2 2021
Could you add a test case for this change? Maybe a unit test?
Aug 5 2021
Aug 2 2021
Jul 29 2021
I think this approach makes sense for now. It's unfortunate that the constructor of FullSourceLoc is unable to validate this requirement, do you know how many clients that you describe as modifying their ID to make them valid are there?
Jul 22 2021
I think this should be fixed in 40d2d0c41298f1d8a178216e2534b29e3128cf37.
I think this should be fixed if I add -fuse-ld= to the invocation. Let me commit that fix to see if it's fixed.
Jul 21 2021
Jul 20 2021
Jul 19 2021
Addressed review comments.
Jul 15 2021
Jul 14 2021
Jul 13 2021
This patch depends on a small NFC commit that moves DarwinSDKInfo over to lib/Basic from lib/Driver, which isn't up on phabricator.
Split patch and address review comments
Sorry, took a bit longer than anticipated. I updated the patch to address reviewer's comments
Jul 8 2021
Looks like this patch causes a number of issues for us, I will work with @jansvoboda11 to see if there's some way to resolve them.
Jun 30 2021
Thanks for the feedback, I'll update this patch tomorrow.
Jun 28 2021
Jun 23 2021
I've started testing this change. I'll let you know how it looks in a few days.
Jun 17 2021
I agree with Duncan, it would be good to avoid modifying the existing test cases if possible
Jun 16 2021
Jun 11 2021
Jun 10 2021
May 24 2021
Hey, thanks for following up on this PR. I've done some more digging and I think we can remove this Darwin-specific workaround in the near future. I'm hoping to provide an update in the next few weeks.
I think this is good to go, I haven't observed any issues with this patch so far in my testing. LGTM
May 19 2021
It might be good for @aganea to take a look as well.
May 14 2021
Do not apply this to 'deprecated' availability annotations - the user has no way to check for deprecated as respondsToSelector will still return true even if the class has marked the method as deprecated.