-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle using nil as argument for andReturn in ObjC++ code. #404
Conversation
de60c8b
to
23c83e7
Compare
ed508f1
to
18214dd
Compare
Anyone have any comments on this one? I think it's a no-brainer. |
Oops.. looks like Erik is looking at it as I type :) Apologies. |
Fix for erikdoe#403. This improves the special case handling of passing nil in return values to handle C++ cases that previously weren't working. Adds tests for C++11 and C++98 variants. To test fully tests need to be executed in both 32bit and 64 bit modes.
Erik, do you have any plans you could share with regards to timing for OCMock 3.7, 3.8 etc? My pile of patches on my end is becoming a little scary :) |
So, the plan was/is to get out 3.7 very soon. I got sidetracked by the PR's regarding Swift Package Manager, but I have now concluded that they won't be in 3.7. So, there's only two open PR's which seem reasonably straight-forward. I was also debating with myself whether #398 should be in, but I haven't read all the comments over there in detail. |
We don't want #398 in its current state IMHO. I would think about it after
#414 was available and see if it is still wanted/needed.
…On Mon, Jul 13, 2020 at 2:04 PM Erik Doernenburg ***@***.***> wrote:
So, the plan was/is to get out 3.7 very soon. I got sidetracked by the
PR's regarding Swift Package Manager, but I have now concluded that they
won't be in 3.7. So, there's only two open PR's which seem reasonably
straight-forward. I was also debating with myself whether #398
<#398> should be in, but I haven't
read all the comments over there in detail.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#404 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACOFSPTASD2U6W3JT2OSGLR3NZFHANCNFSM4MKFOT2A>
.
|
@@ -547,7 +551,7 @@ | |||
2FA28006D043CBDBBAEF6E3F /* OCMMacroState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OCMMacroState.h; sourceTree = "<group>"; }; | |||
2FA280987F4EA8A4D79000D0 /* OCMMacroState.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMMacroState.m; sourceTree = "<group>"; }; | |||
2FA280EB5E8CDEEAE76861F7 /* OCMNonRetainingObjectReturnValueProvider.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMNonRetainingObjectReturnValueProvider.m; sourceTree = "<group>"; }; | |||
2FA2813F93050582D83E1499 /* OCMockObjectRuntimeTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMockObjectRuntimeTests.m; sourceTree = "<group>"; }; | |||
2FA2813F93050582D83E1499 /* OCMockObjectRuntimeTests.m */ = {isa = PBXFileReference; explicitFileType = sourcecode.cpp.objcpp; fileEncoding = 4; path = OCMockObjectRuntimeTests.m; sourceTree = "<group>"; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change breaks the build for me. When I change the file type back to Objective-C it works. The error is:
/Users/erik/Projects/OCMock/Repositories/ocmock/Source/OCMockTests/OCMockObjectRuntimeTests.m:219:54: error: cannot initialize a parameter of type 'void *' with an lvalue of type 'const char [4]'
XCTAssertNoThrow([[myMock expect] aSpecialMethod:"foo"], @"Should not complain about method with type qualifiers.");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh.. that is probably an accidental mod when I changed the file to objc++ before I broke the tests out into their own files. No need to pull that across. Apologies.
XCTAssertTrue([boxed isMethodReturnType:type2 compatibleWithValueType:type3]); | ||
XCTAssertTrue([boxed isMethodReturnType:type3 compatibleWithValueType:type1]); | ||
XCTAssertTrue([boxed isMethodReturnType:type3 compatibleWithValueType:type2]); | ||
XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2 value:&value valueSize:valueSize]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed these tests to pass NULL
and 0
for the last two arguments. Couldn't see a reason why not, and it saved creating the value in the test.
Fix for #403.
This improves the special case handling of passing nil in return values to handle
C++ cases that previously weren't working.
Adds tests for C++11 and C++98 variants. To test fully tests need to be executed in both
32bit and 64 bit modes.