-
-
Notifications
You must be signed in to change notification settings - Fork 925
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Truffle] Better demonstrator of using new SnippetNode.
- 9.4.12.0
- 9.4.11.0
- 9.4.10.0
- 9.4.9.0
- 9.4.8.0
- 9.4.7.0
- 9.4.6.0
- 9.4.5.0
- 9.4.4.0
- 9.4.3.0
- 9.4.2.0
- 9.4.1.0
- 9.4.0.0
- 9.3.15.0
- 9.3.14.0
- 9.3.13.0
- 9.3.12.0
- 9.3.11.0
- 9.3.10.0
- 9.3.9.0
- 9.3.8.0
- 9.3.7.0
- 9.3.6.0
- 9.3.5.0
- 9.3.4.0
- 9.3.3.0
- 9.3.2.0
- 9.3.1.0
- 9.3.0.0
- 9.2.21.0
- 9.2.20.1
- 9.2.20.0
- 9.2.19.0
- 9.2.18.0
- 9.2.17.0
- 9.2.16.0
- 9.2.15.0
- 9.2.14.0
- 9.2.13.0
- 9.2.12.0
- 9.2.11.1
- 9.2.11.0
- 9.2.10.0
- 9.2.9.0
- 9.2.8.0
- 9.2.7.0
- 9.2.6.0
- 9.2.5.0
- 9.2.4.1
- 9.2.4.0
- 9.2.3.0
- 9.2.2.0
- 9.2.1.0
- 9.2.0.0
- 9.1.17.0
- 9.1.16.0
- 9.1.15.0
- 9.1.14.0
- 9.1.13.0
- 9.1.12.0
- 9.1.11.0
- 9.1.10.0
- 9.1.9.0
- 9.1.8.0
- 9.1.7.0
- 9.1.6.0
- 9.1.5.0
- 9.1.4.0
- 9.1.3.0
- 9.1.2.0
- 9.1.1.0
- 9.1.0.0
1 parent
c2f9145
commit f6a2f67
Showing
1 changed file
with
11 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jruby/truffle here's better example of using the new
SnippetNode
.f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure, I think I liked the
createSnippet()
version more, because the arguments were not substituted with the constants.f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does everyone else think?
Maybe we could write:
f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current version or a
createSnippet
helper both look fine to me. I think the$
would be too much magic.f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I agree with Petr. But I generally find it annoying to track down where the code in a @cached fragment lives. The old solution looked like it could be adopted into a pattern of sorts and then I'd always know where to look as long as the pattern is followed.
f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to always put the final fields next to the specialisation, not at the top of the class. The reason I don't like Petr's preferred option is that it adds another method when we just got the ability to not have to do that (with no context or section)!
f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly enough one way or the other. I'm just concerned our runtime is getting to be too stringly typed.
f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering all the options:
the 5. option seems best to me.
f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was also thinking to something like 5, where essentially we just add
snippet.
beforeruby(...
but then the constraint that the code and arguments must be fixed is slightly unclear.Another alternative is to standardize argument names to for instance
argN
, then we only need a constant for the code (similar to 1).f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it's unclear but it can be checked to catch mis-use (when compiled and used correctly the check should be eliminated I think, otherwise it could be checked only on interpreted path). 1 with argN seems good too (I still slightly prefer 5 though.)
f6a2f67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could live with 5. Everyone happy with that?