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

[Truffle] Adding annotation to return an enumerator if no block is given to a method. #2720

Merged

Conversation

bjfish
Copy link
Contributor

@bjfish bjfish commented Mar 18, 2015

No description provided.

final RubyProc block = RubyArguments.getBlock(frame.getArguments());

if (block == null) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to be a pain, but we don't want a blank line here...

@bjfish bjfish force-pushed the truffle_return_enumerator_annotation branch from 76bf07a to eed837f Compare March 18, 2015 17:50
@bjfish
Copy link
Contributor Author

bjfish commented Mar 18, 2015

@chrisseaton I updated but wasn't sure if i got the adding and removing lines correct

chrisseaton added a commit that referenced this pull request Mar 18, 2015
…tion

[Truffle] Adding annotation to return an enumerator if no block is given to a method.
@chrisseaton chrisseaton merged commit e454d1c into jruby:master Mar 18, 2015
@chrisseaton chrisseaton added this to the truffle-dev milestone Mar 18, 2015
public Object execute(VirtualFrame frame) {
final RubyProc block = RubyArguments.getBlock(frame.getArguments());

if (block == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want a ConditionProfile here. We've essentially collapsed two specializations into one to consolidate a common pattern. But a given call site is apt to only use one form or the other.

@bjfish
Copy link
Contributor Author

bjfish commented Mar 18, 2015

@chrisseaton it would be nice if the style things could be checked with something like checkstyle

Also, I have had trouble using raiseIfFrozen or the IsFrozenNode in a method after the method is to_enum'ed. Any ideas? I might try to debug this later on.


} else {
try {
return method.executeRubyBasicObject(frame);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be execute instead of executeRubyBasicObject. I'm not sure how often it happens (maybe never), but this wrapper won't work if the specialization returns a primitive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sorry these notes are both correct and important but I already merged - can submit another PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll submit a new PR

@enebo enebo added this to the Non-Release milestone Dec 7, 2017
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.

None yet

4 participants