hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.39k stars 232 forks source link

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

Closed JohelEGP closed 8 months ago

JohelEGP commented 8 months ago

Resolves #882.

Testing summary:

100% tests passed, 0 tests failed out of 858

Total Test time (real) =  44.64 sec

Acknowledgements:

hsutter commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

Thanks!