- User Since
- Jun 25 2014, 4:17 PM (290 w, 2 d)
Fri, Jan 10
LGTM, with a nit mentioned.
Thu, Jan 9
It might be hard to test in a regular build: you probably would need to construct a test-case where the buffer is precisely a multiple of the page size and not nil-terminated, and then hopefully the OS will trigger a fault once you access the out of bounds character. Have you tried ASANified build? I think it could trigger a crash even if you don't get the pages right and just have a not-nil terminated buffer.
Wed, Jan 8
Did you check if you can reproduce it with a unittest by passing in a Buffer that's not nil-terminated?
Dec 17 2019
Looks like HOST_LINK_VERSION is influencing the decision. I should specify the version manually in the tests then.
@phosek Thanks, I think it reproduces on our macOS bots. I'll start fixing it right now.
I couldn't come up with a reasonable Sema-based test, so I'm going to commit this as it is.
LGTM, But we should also have a test in Sema that verifies it works, and also covers class properties in addition to the instance ones. I can add one when committing (@MadCoder still doesn't have commit access).
Dec 16 2019
Dec 12 2019
Dec 10 2019
Dec 9 2019
Dec 6 2019
@MadCoder unfortunately this change causes several test failures when check-clang runs:
LGTM, after fixing JF's comments. Please upload patches with full context as JF mentioned, as it helps with the review.
Dec 5 2019
@MadCoder Indexing by names doesn't seem like the right solution. You should be able to get the canonical declaration (getCanonicalDecl) from the declaration that represents the method in @implementation, which should avoid the problem you're describing.
The environment variable should be enough IMHO
I guess Duncan's comments on https://reviews.llvm.org/D70568 were for this patch. I think it would be good to have: