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

small change in copyWithoutReturn to handle CCoerce #836

Merged

Conversation

RenaudFondeur
Copy link
Contributor

@RenaudFondeur RenaudFondeur commented Jul 25, 2024

Suppress cCoerce at the top of return expression when we suppress return in inlining.

cCoerce can be found in returns to make them match their method return type.
During inlining we can create a copy of parts of the AST without their return. If there is a cCoerce at the top of their expression, it may be no longer needed and can produce a 'warning: expression result unused'.

Copy link
Collaborator

@PalumboN PalumboN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🆒 !

I suggest to add a comment and any test.
I don't remember well, but part of the inlinings are tested. Would be nice to add one example of this.

Also, I did not get why the compiler throws warning: expression result unused due a cast 🤔 (maybe the test helps me to understand).

smalltalksrc/Slang/TReturnNode.class.st Outdated Show resolved Hide resolved
Copy link
Collaborator

@PalumboN PalumboN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cooollll! This is better, mainly because of the tests 💯 🚀 🚦

I left new comments / questions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤩

smalltalksrc/Slang/TMethod.class.st Show resolved Hide resolved
smalltalksrc/Slang/TParseNode.class.st Show resolved Hide resolved
@guillep guillep merged commit 65d2cab into pharo-project:pharo-10 Aug 28, 2024
1 of 2 checks passed
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.

3 participants