{"action":"created","issue":{"url":"https://api.github.com/repos/elastic/Painless/issues/13","labels_url":"https://api.github.com/repos/elastic/Painless/issues/13/labels{/name}","comments_url":"https://api.github.com/repos/elastic/Painless/issues/13/comments","events_url":"https://api.github.com/repos/elastic/Painless/issues/13/events","html_url":"https://github.com/elastic/Painless/issues/13","id":112854085,"number":13,"title":"Use GeneratorAdaptor instead of plain MethodVisitor to generate code","user":{"login":"uschindler","id":1005388,"avatar_url":"https://avatars.githubusercontent.com/u/1005388?v=3","gravatar_id":"","url":"https://api.github.com/users/uschindler","html_url":"https://github.com/uschindler","followers_url":"https://api.github.com/users/uschindler/followers","following_url":"https://api.github.com/users/uschindler/following{/other_user}","gists_url":"https://api.github.com/users/uschindler/gists{/gist_id}","starred_url":"https://api.github.com/users/uschindler/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uschindler/subscriptions","organizations_url":"https://api.github.com/users/uschindler/orgs","repos_url":"https://api.github.com/users/uschindler/repos","events_url":"https://api.github.com/users/uschindler/events{/privacy}","received_events_url":"https://api.github.com/users/uschindler/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-10-22T17:41:59Z","updated_at":"2015-10-22T17:58:29Z","closed_at":null,"body":"I just reviewed the code. In contrast to Lucene expressions module the new code does not use the very nice GeneratorAdaptor wrapper in the bytecode generator. Instead it push pops and calls methods in the most messy plain bytecode stack machine way all the time. Lots of code duplication for calling methods, much boilerplate for invokeInsn calls and so on. It would be much better to wrap the MethodVisitor with GeneratorAdaptor (from asm-commons package) and use that convenience methods to generate stuff like methods calls, bi-operators, loops/jumps in a much more readable way. The current code is very hard to read. If you use GeneratorAdaptor you can incrementally convert the code to use the easy-use methods. GeneratorAdaptor is still a methodVisitor but with more utility methods. This would reduce the ASM-generator code by a significant part."},"comment":{"url":"https://api.github.com/repos/elastic/Painless/issues/comments/150306447","html_url":"https://github.com/elastic/Painless/issues/13#issuecomment-150306447","issue_url":"https://api.github.com/repos/elastic/Painless/issues/13","id":150306447,"user":{"login":"jdconrad","id":2126764,"avatar_url":"https://avatars.githubusercontent.com/u/2126764?v=3","gravatar_id":"","url":"https://api.github.com/users/jdconrad","html_url":"https://github.com/jdconrad","followers_url":"https://api.github.com/users/jdconrad/followers","following_url":"https://api.github.com/users/jdconrad/following{/other_user}","gists_url":"https://api.github.com/users/jdconrad/gists{/gist_id}","starred_url":"https://api.github.com/users/jdconrad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdconrad/subscriptions","organizations_url":"https://api.github.com/users/jdconrad/orgs","repos_url":"https://api.github.com/users/jdconrad/repos","events_url":"https://api.github.com/users/jdconrad/events{/privacy}","received_events_url":"https://api.github.com/users/jdconrad/received_events","type":"User","site_admin":false},"created_at":"2015-10-22T17:58:29Z","updated_at":"2015-10-22T17:58:29Z","body":"Thanks for reviewing this.  I'm all for making the code better -- 2 questions about generator adapter and ASM in general.\r\n\r\n1) If we want to support type checking for something like a Map<String, Object> at compile time, it wasn't clear to me if there was a good way to use ASM types to do this.  What did I miss here?\r\n2) Maybe this doesn't matter, but it looked to me like when I was adding variables to the stack using a GeneratorAdapter it wasn't tracking scope for things like loops and if statements, so it would just keep increasing the stack size when it didn't need to.  Was I doing something incorrectly here?"}}