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

[Decompiler][Patch]Failed around TupleGet pattern #77

Closed
GoogleCodeExporter opened this issue Jun 21, 2015 · 9 comments
Closed

[Decompiler][Patch]Failed around TupleGet pattern #77

GoogleCodeExporter opened this issue Jun 21, 2015 · 9 comments

Comments

@GoogleCodeExporter
Copy link

I'm going to user your library in my project https://github.com/Kakadu/Falka/
Your tool have generatored worng code for
fun a b c ->
          match (a,b) with
          | (TNumber a,TOperator x) -> AExpr (x, ANumber a,c)
          | _ -> failwith "some bug here")

because you haven't investigate TupleGet pattern while decompilation,

Patch added.



Original issue reported on code.google.com by kakadu.hafanana on 1 Feb 2012 at 11:10

Attachments:

@GoogleCodeExporter
Copy link
Author

Kakadu: thank you for the report and especially the supplied patch! Sorry it's 
taken me a little while to follow up on this, google code didn't seem to notify 
me and I only happened to look at the issue list right now and spot it. I will 
follow up on this and hopefully get a release for you in a timely manner.

Original comment by stephen....@gmail.com on 10 Feb 2012 at 7:42

@GoogleCodeExporter
Copy link
Author

Original comment by stephen....@gmail.com on 10 Feb 2012 at 7:42

@GoogleCodeExporter
Copy link
Author

Kakadu - can you give me a simpler, self-contained example of the code you are 
quoting, the decompiled result you are seeing, and the decompiled result you 
expect?

Please note that TupleGet is one of the more complicated, context driven 
quotation patterns which I have battled against mightily and created an 
abstraction around: 
http://code.google.com/p/unquote/source/browse/trunk/Unquote/ExtraPatterns.fs?r=
468#105

While I appreciate the patch you supplied, it is not as robust as the TupleLet 
pattern, and depends on the assumption of an implementation of the (?) dynamic 
operator.

Original comment by stephen....@gmail.com on 11 Apr 2012 at 1:44

@GoogleCodeExporter
Copy link
Author

Try with this code.

> let f = <@ fun (a:int) (b:int) -> match a,b with | (1,_) -> 1  | _ -> b@> ;;
val f : Quotations.Expr<(int -> int -> int)> =
  Lambda (a,
        Lambda (b,
                Let (matchValue, NewTuple (a, b),
                     IfThenElse (Call (None,
                                       Boolean op_Equality[Int32](Int32, Int32),
                                       [TupleGet (matchValue, 0), Value (1)]),
                                 Value (1), b))))

> Unquote.Operators.decompile (f );;
val it : string =
  "fun a b -> let matchValue = (a, b) in if TupleGet (matchValue, 0) = 1 then 1 else b"

I dont expect to see TupleGet in the result. AFAIU it goes to the result in 
last pattern matching with wildcard. It seems that you are expecting correct 
pattern matching case in Decompilation.fs+227.


Original comment by kakadu.hafanana on 11 Apr 2012 at 7:50

@GoogleCodeExporter
Copy link
Author

Perfect example: thank you!

Absolutely you shouldn't see TupleGet in the result: my goal is to approach 
100% decompiling to valid code (the decompiler falls back on F#'s quotation 
printing in cases it fails to consider, as you are seeing here).

But, do note that some expressions require very hard or near impossible 
"resugaring". In particular, the semantic forms of sequence expressions and 
matches are very different from their syntactic forms. So, in this example, the 
best I can do is probably

"fun a b -> let matchValue = (a, b) in if (let (t1,_) = matchValue in t1) = 1 
then 1 else b"

where I am falling back on using (let (t1,_) = matchValue in t1) instead of the 
dynamic approach you suggested, because, even though my version would be more 
verbose, I do not want to assume any outside dependencies such as the existing 
of a dynamic operator which works correctly on tuples.

Original comment by stephen....@gmail.com on 11 Apr 2012 at 2:31

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Author

So we can close this bug.

Original comment by kakadu.hafanana on 11 Apr 2012 at 2:46

@GoogleCodeExporter
Copy link
Author

Once I implement: yes. But I haven't done the work yet. I expect to fix this 
and get a release out including other bug fixes and enhancements by the end of 
the week.

Original comment by stephen....@gmail.com on 11 Apr 2012 at 3:10

@GoogleCodeExporter
Copy link
Author

This issue was closed by revision r474.

Original comment by stephen....@gmail.com on 11 Apr 2012 at 7:10

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Original comment by stephen....@gmail.com on 11 Apr 2012 at 8:46

  • Added labels: Milestone-Release2.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant