Skip to content

Conversation

@FranckRJ
Copy link
Collaborator

@FranckRJ FranckRJ commented Oct 5, 2025

Basically what's here: #337, will write something better later.

FranckRJ added 7 commits June 1, 2025 00:30
…returning a reference, and improved ReturnCapture to construct the return type from the parameter if their reference is not compatible (to allow things like function-returning-std-string.ReturnCapture("string")).
…eference (in a deprecated manner), and other minor fixes.
@FranckRJ FranckRJ added this to the 2.5.0 milestone Oct 5, 2025
@coveralls
Copy link

coveralls commented Oct 5, 2025

Coverage Status

coverage: 99.566% (+0.003%) from 99.563%
when pulling 0d2e906 on make-return-with-spe-works-again-and-bonuses
into bd11787 on dev.

@FranckRJ
Copy link
Collaborator Author

FranckRJ commented Oct 5, 2025

Hello @otrempe and @malcolmdavey, from what I understood from things you said in other issues, you have some codebases that use explicit template specializations for .Return(), like .Return<std::string>(val) to capture the value by copy for a function that returns a reference, or .Return<std::string&>(val) to capture the value by reference for a function that returns a copy.

If you can, I would like you to test this PR with your codebase (I already generated the single headers here: https://github.com/eranpeer/FakeIt/tree/make-return-with-spe-works-again-and-bonuses/single_header), because previous versions of FakeIt 2.5.0-dev disabled this (undocumented) feature, but I tried to add it back, and I would like to know if it works for more complexes use cases than the ones I come up in the unit tests.

I would really like to be able to release 2.5.0 without breaking anyone's code, if anyone can help I would be really grateful.

@malcolmdavey
Copy link
Contributor

Hello @otrempe and @malcolmdavey, from what I understood from things you said in other issues, you have some codebases that use explicit template specializations for .Return(), like .Return<std::string>(val) to capture the value by copy for a function that returns a reference, or .Return<std::string&>(val) to capture the value by reference for a function that returns a copy.

If you can, I would like you to test this PR with your codebase (I already generated the single headers here: https://github.com/eranpeer/FakeIt/tree/make-return-with-spe-works-again-and-bonuses/single_header), because previous versions of FakeIt 2.5.0-dev disabled this (undocumented) feature, but I tried to add it back, and I would like to know if it works for more complexes use cases than the ones I come up in the unit tests.

I would really like to be able to release 2.5.0 without breaking anyone's code, if anyone can help I would be really grateful.

Hi Frank.

As it turns out I hadn't actually used any cases of explicit types yet, but had just added unit tests for fakeit to give more complete coverage.
The main case I was after was being able to construct a value inline, where it will be returned as a reference, and then I need to increase it's lifetime.
This was the only case where I've found a compile error (by design) - though this would also have been affected by the previous merged PR (#332 )

My existing code (using fakeit 2.4 ?)

	When(Method(object, method)).AlwaysReturn({part1, part2});

Now compared to AlwaysReturn/Return, since it doesn't know the type, I need to make the type explicit
by either of these:

	When(Method(object, method)).AlwaysReturnValCapt(MyType{part1, part2});
	When(Method(object, method)).AlwaysReturnValCapt<MyType>({ part1, part2 }});

I guess this is less ideal compared to the behaviour of Return/AlwaysReturn, but it isn't a big deal.
If it's simple to fix that would be great though, assuming there is no down side to it.

@malcolmdavey
Copy link
Contributor

malcolmdavey commented Oct 9, 2025

Also found an unrelated compile error here that I posted here
#357

@FranckRJ
Copy link
Collaborator Author

FranckRJ commented Oct 9, 2025

I didn't thought about that, yeah adding a default type for the template so you don't need to explicitly provide one is easy, I tried on Godbolt and it works, I'll change that. Thank you for the suggestion !

Thanks for reporting the bug with msvc, I'll fix that as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants