Fall back on CoreFoundation for some core NSString/NSCFString behaviours #671
Conversation
|
@ms-jihua is added to the review. #Closed |
|
@bbowman is added to the review. #Closed |
| StringsFormatPropertyList, | ||
| ::testing::Values(L"\uFEFFkey1=value1;\n\"key2\"=\"value2\";", // BOM | ||
| L"key1=value1;\n\"key2\"=\"value2\";" // No BOM | ||
| )); |
There was a problem hiding this comment.
Glad you think so! I used some of the other NSString tests as inspiration.
In reply to: 71446495 [](ancestors = 71446495)
|
|
| TEST(NSString, Exceptions) { | ||
| NSRange range{ 100, 10 }; | ||
|
|
||
| EXPECT_ANY_THROW([@"hello" characterAtIndex:10]); |
There was a problem hiding this comment.
EXPECT_ANY_THROW [](start = 4, length = 16)
Do these tests work on ARM? #Closed
There was a problem hiding this comment.
I've been caught! Probably not. I'll throw them into ARM_DISABLED_TESTS.
In reply to: 71574294 [](ancestors = 71574294)
There was a problem hiding this comment.
These tests actually do work on ARM, puzzlingly.
In reply to: 71574787 [](ancestors = 71574787,71574294)
There was a problem hiding this comment.
Thanks for checking. A wise man once said "Its always good to ask".
In reply to: 71600230 [](ancestors = 71600230,71574787,71574294)
|
|
DHowett-MSFT
force-pushed
the
201605-CFString-internal
branch
from
acc5804 to
a8a1587
Compare
|
In CI Build! |
DHowett-MSFT
force-pushed
the
201605-CFString-internal
branch
from
a8a1587 to
5025d2d
Compare
|
Failed an application launch test. Addressing in an additional commit. |
| } | ||
|
|
||
| if (self.length >= 2) { | ||
| return _isLetter([self characterAtIndex:0]) && [self characterAtIndex:1] == ':'; |
There was a problem hiding this comment.
This function is technically wrong, because C:Hello.txt is a relative path, not an absolute one.
Fixing it is out-of-scope for this change.
There was a problem hiding this comment.
Talked to DHowett offline, he's going to take a closer look at drive paths next iteration, and leave this slightly-wrong state for now.
Be sure to add a comment about this status.
| @@ -537,6 +537,10 @@ - (BOOL)getBytes:(void*)buffer | |||
| @Status Interoperable | |||
| */ | |||
| - (void)getCharacters:(unichar*)dest range:(NSRange)range { | |||
| if (range.location + range.length > self.length) { | |||
There was a problem hiding this comment.
surprised we didn't have this check already. use NSLocationInRange instead, though?
There was a problem hiding this comment.
We don't exactly want that here; if length is 5, NSLocationInRange(5, {1,2}) will be false but that doesn't make this invalid.
|
Submitting a new CI build with the updates, just to make sure. |
|
Passed CI build and application launch tests. |
This reimplements some of
_NSCFString's internals in terms of non-backstore-checking CF internal implementations exposed in ForFoundationOnly.h.As a result, we now throw range/length exceptions and properly catch string mutability without exploding.
It also replaces our home-grown strings format parser with CoreFoundation's tried/true one.
Additionally, printing an object via NSString's
%@format specifier will prefer-descriptioninstead ofCFShow()for Objective-C types.