Skip to content
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

fix(to_cpp1): don't emit [[nodiscard]] for pre-increment/decrement #883

Merged

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Dec 9, 2023

Resolves #882.

Testing summary:

100% tests passed, 0 tests failed out of 858

Total Test time (real) =  44.64 sec

Acknowledgements:

@hsutter
Copy link
Owner

hsutter commented Dec 10, 2023

Thanks! I think just the name operator++ and operator-- should be enough, because there should be no other signature allowed... I am now updating this PR to add a sema check to enforce that.

Also I'll remove the word "prefix" to avoid confusion because ++/-- are only postfix in Cpp2, though they lower to Cpp1 prefix operators.

@JohelEGP
Copy link
Contributor Author

Thanks! I think just the name operator++ and operator-- should be enough, because there should be no other signature allowed... I am now updating this PR to add a sema check to enforce that.

Do you mean in function_type_node::is_increment_or_decrement?
There's the _: int signature for post-increment for Cpp1 compatibility.
We want those to continue to be [[nodiscard]].

operator++: (inout this, _: int) …

@hsutter
Copy link
Owner

hsutter commented Dec 10, 2023

Do you mean in function_type_node::is_increment_or_decrement?

Yes, I was going to rename the new function to that (from is_prefix_increment_or_decrement).

There's the _: int signature for post-increment for Cpp1 compatibility.
We want those to continue to be [[nodiscard]].

Ah, that's an oversight -- I don't think I noticed that you could still author that. My mental model was that Cpp2 has only postfix increment/decrement with in-place semantics, but now that I'm looking at your note, I think the right answer is Cpp2 should generate both Cpp1 versions (prefix and postfix, the latter with [[nodiscard]]) from the Cpp2 version. That way a Cpp2 type still has the correct simple mental model, but Cpp1 call sites are still able to use postfix for full compatibility.

@hsutter hsutter merged commit d7ea6c0 into hsutter:main Dec 10, 2023
@JohelEGP JohelEGP deleted the not_nodiscard_pre_increment_and_decrement branch December 10, 2023 19:52
@hsutter
Copy link
Owner

hsutter commented Dec 10, 2023

Thanks!

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.

[BUG] Emitted prefix increment and decrement should not be [[nodiscard]]
2 participants