Skip to content

Commit

Permalink
[clang] fix evaluation order bug discovered by building with OCaml 4.…
Browse files Browse the repository at this point in the history
…05.0

Summary:
The behaviour of infer was observed to be different in 4.04.2 vs (4.05.0 or
4.04.2+flambda). Investigating further, the behaviour is also different in byte
vs native versions of infer under 4.04.2. Looking at the Changelog of 4.05.0
(thanks mbouaziz), there are fixes for inconsistent behaviours between byte
and native relative to evaluation order.

Lots of debugging later, this 1 line patch is born. Also use `List.concat_map`
instead of re-implementing it.

Reviewed By: jberdine

Differential Revision: D5793809

fbshipit-source-id: 374fb4c
  • Loading branch information
jvillard authored and facebook-github-bot committed Sep 8, 2017
1 parent 740e997 commit 5b3c2c0
Showing 1 changed file with 12 additions and 15 deletions.
27 changes: 12 additions & 15 deletions infer/src/clang/cTrans.ml
Expand Up @@ -173,21 +173,18 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
(Exp.Var id, typ)
in
let make_arg typ (id, _, _) = (id, typ) in
let rec f es =
match es with
| []
-> []
| (Exp.Closure {name; captured_vars}, ({Typ.desc= Tptr ({Typ.desc= Tfun _}, _)} as t)) :: es'
-> let app =
let function_name = make_function_name t name in
let args = List.map ~f:(make_arg t) captured_vars in
function_name :: args
in
app @ f es'
| e :: es'
-> e :: f es'
in
(f exps, !insts)
let f = function
| Exp.Closure {name; captured_vars}, ({Typ.desc= Tptr ({Typ.desc= Tfun _}, _)} as t)
-> let function_name = make_function_name t name in
let args = List.map ~f:(make_arg t) captured_vars in
function_name :: args
| e
-> [e]
in
(* evaluation order matters here *)
let exps' = List.concat_map ~f exps in
let insts' = !insts in
(exps', insts')

let collect_exprs res_trans_list =
List.concat_map ~f:(fun res_trans -> res_trans.exps) res_trans_list
Expand Down

1 comment on commit 5b3c2c0

@mbouaziz
Copy link
Contributor

Choose a reason for hiding this comment

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

@jmid here is Jules' fix we talked about on Friday

Please sign in to comment.