Skip to content

Commit

Permalink
Fixing yet a source of dependency on alphabetic order in unification.
Browse files Browse the repository at this point in the history
This refines even further c24bcae (PR #924) and 6304c84:
- c24bcae fixed the order in the heuristic
- 6304c84 improved the order by preferring projections

There remained a dependency in the alphabetic order in selecting
unification candidates. The current commit fixes it.

We radically change the representation of the substitution to invert
by using a map indexed on the rank in the signature rather than on the
name of the variable.

More could be done to use numbers further, e.g. for representing
aliases.

Note that this has consequences on the test-suite (in
output/Notations.v) as some problems now infer a dependent return
clause.
  • Loading branch information
herbelin committed Apr 16, 2018
1 parent 1278017 commit ee912f7
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 49 deletions.
2 changes: 1 addition & 1 deletion pretyping/cases.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1717,7 +1717,7 @@ let abstract_tycon ?loc env evdref subst tycon extenv t =
List.map (fun d -> local_occur_var !evdref (NamedDecl.get_id d) u)
(named_context extenv) in
let filter = Filter.make (rel_filter @ named_filter) in
let candidates = u :: List.map mkRel vl in
let candidates = List.rev (u :: List.map mkRel vl) in
let ev = e_new_evar extenv evdref ~src ~filter ~candidates ty in
lift k ev
in
Expand Down
3 changes: 2 additions & 1 deletion pretyping/evarconv.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,7 @@ let rec solve_unconstrained_evars_with_candidates ts evd =
let rec aux = function
| [] -> user_err Pp.(str "Unsolvable existential variables.")
| a::l ->
(* In case of variables, most recent ones come first *)
try
let conv_algo = evar_conv_x ts in
let evd = check_evar_instance evd evk a conv_algo in
Expand All @@ -1330,7 +1331,7 @@ let rec solve_unconstrained_evars_with_candidates ts evd =
| e when Pretype_errors.precatchable_exception e -> aux l in
(* List.rev is there to favor most dependent solutions *)
(* and favor progress when used with the refine tactics *)
let evd = aux (List.rev l) in
let evd = aux l in
solve_unconstrained_evars_with_candidates ts evd

let solve_unconstrained_impossible_cases env evd =
Expand Down
68 changes: 27 additions & 41 deletions pretyping/evarsolve.ml
Original file line number Diff line number Diff line change
Expand Up @@ -599,34 +599,39 @@ let solve_pattern_eqn env sigma l c =
let make_projectable_subst aliases sigma evi args =
let sign = evar_filtered_context evi in
let evar_aliases = compute_var_aliases sign sigma in
let (_,full_subst,cstr_subst) =
List.fold_right
(fun decl (args,all,cstrs) ->
let (_,full_subst,cstr_subst,_) =
List.fold_right_i
(fun i decl (args,all,cstrs,revmap) ->
match decl,args with
| LocalAssum (id,c), a::rest ->
let revmap = Id.Map.add id i revmap in
let cstrs =
let a',args = decompose_app_vect sigma a in
match EConstr.kind sigma a' with
| Construct cstr ->
let l = try Constrmap.find (fst cstr) cstrs with Not_found -> [] in
Constrmap.add (fst cstr) ((args,id)::l) cstrs
| _ -> cstrs in
(rest,Id.Map.add id [a,normalize_alias_opt sigma aliases a,id] all,cstrs)
let all = Int.Map.add i [a,normalize_alias_opt sigma aliases a,id] all in
(rest,all,cstrs,revmap)
| LocalDef (id,c,_), a::rest ->
let revmap = Id.Map.add id i revmap in
(match EConstr.kind sigma c with
| Var id' ->
let idc = normalize_alias_var sigma evar_aliases id' in
let sub = try Id.Map.find idc all with Not_found -> [] in
let ic, sub =
try let ic = Id.Map.find idc revmap in ic, Int.Map.find ic all
with Not_found -> i, [] (* e.g. [idc] is a filtered variable: treat [id] as an assumption *) in
if List.exists (fun (c,_,_) -> EConstr.eq_constr sigma a c) sub then
(rest,all,cstrs)
(rest,all,cstrs,revmap)
else
(rest,
Id.Map.add idc ((a,normalize_alias_opt sigma aliases a,id)::sub) all,
cstrs)
let all = Int.Map.add ic ((a,normalize_alias_opt sigma aliases a,id)::sub) all in
(rest,all,cstrs,revmap)
| _ ->
(rest,Id.Map.add id [a,normalize_alias_opt sigma aliases a,id] all,cstrs))
| _ -> anomaly (Pp.str "Instance does not match its signature."))
sign (Array.rev_to_list args,Id.Map.empty,Constrmap.empty) in
let all = Int.Map.add i [a,normalize_alias_opt sigma aliases a,id] all in
(rest,all,cstrs,revmap))
| _ -> anomaly (Pp.str "Instance does not match its signature.")) 0
sign (Array.rev_to_list args,Int.Map.empty,Constrmap.empty,Id.Map.empty) in
(full_subst,cstr_subst)

(*------------------------------------*
Expand Down Expand Up @@ -793,11 +798,11 @@ let rec assoc_up_to_alias sigma aliases y yc = function

let rec find_projectable_vars with_evars aliases sigma y subst =
let yc = normalize_alias sigma aliases y in
let is_projectable idc idcl subst' =
let is_projectable idc idcl (subst1,subst2 as subst') =
(* First test if some [id] aliased to [idc] is bound to [y] in [subst] *)
try
let id = assoc_up_to_alias sigma aliases y yc idcl in
(id,ProjectVar)::subst'
(id,ProjectVar)::subst1,subst2
with Not_found ->
(* Then test if [idc] is (indirectly) bound in [subst] to some evar *)
(* projectable on [y] *)
Expand All @@ -812,14 +817,18 @@ let rec find_projectable_vars with_evars aliases sigma y subst =
let subst,_ = make_projectable_subst aliases sigma evi argsv in
let l = find_projectable_vars with_evars aliases sigma y subst in
match l with
| [id',p] -> (id,ProjectEvar (t,evi,id',p))::subst'
| [id',p] -> (subst1,(id,ProjectEvar (t,evi,id',p))::subst2)
| _ -> subst'
end
| [] -> subst'
| _ -> anomaly (Pp.str "More than one non var in aliases class of evar instance.")
else
subst' in
Id.Map.fold is_projectable subst []
let subst1,subst2 = Int.Map.fold is_projectable subst ([],[]) in
(* We return the substitution with ProjectVar first (from most
recent to oldest var), followed by ProjectEvar (from most recent
to oldest var too) *)
subst1 @ subst2

(* [filter_solution] checks if one and only one possible projection exists
* among a set of solutions to a projection problem *)
Expand All @@ -842,25 +851,6 @@ let rec find_solution_type evarenv = function
| (id,ProjectEvar _)::l -> find_solution_type evarenv l
| [] -> assert false

let is_preferred_projection_over sign (id,p) (id',p') =
(* We give priority to projection of variables over instantiation of
an evar considering that the latter is a stronger decision which
may even procude an incorrect (ill-typed) solution *)
match p, p' with
| ProjectEvar _, ProjectVar -> false
| ProjectVar, ProjectEvar _ -> true
| _, _ ->
List.index Id.equal id sign < List.index Id.equal id' sign

let choose_projection evi sols =
let sign = List.map get_id (evar_filtered_context evi) in
match sols with
| y::l ->
List.fold_right (fun (id,p as x) (id',_ as y) ->
if is_preferred_projection_over sign x y then x else y)
l y
| _ -> assert false

(* In case the solution to a projection problem requires the instantiation of
* subsidiary evars, [do_projection_effects] performs them; it
* also try to instantiate the type of those subsidiary evars if their
Expand Down Expand Up @@ -1444,12 +1434,8 @@ let rec invert_definition conv_algo choose env evd pbty (evk,argsv as ev) rhs =
let c, p = match sols with
| [] -> raise Not_found
| [id,p] -> (mkVar id, p)
| _ ->
if choose then
let (id,p) = choose_projection evi sols in
(mkVar id, p)
else
raise (NotUniqueInType sols)
| (id,p)::_ ->
if choose then (mkVar id, p) else raise (NotUniqueInType sols)
in
let ty = lazy (Retyping.get_type_of env !evdref (of_alias t)) in
let evd = do_projection_effects (evar_define conv_algo ~choose) env ty !evdref p in
Expand Down
8 changes: 8 additions & 0 deletions test-suite/bugs/closed/2670.v
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ Proof.
refine (match e return _ with refl_equal => _ end).
reflexivity.
Undo 2.
(** Check insensitivity to alphabetic order *)
refine (match e as a in _ = b return _ with refl_equal => _ end).
reflexivity.
Undo 2.
(** Check insensitivity to alphabetic order *)
refine (match e as z in _ = y return _ with refl_equal => _ end).
reflexivity.
Undo 2.
(* Next line similarly has a dependent and a non dependent solution *)
refine (match e with refl_equal => _ end).
reflexivity.
Expand Down
14 changes: 8 additions & 6 deletions test-suite/output/Notations.out
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,15 @@ s
: nat
fun _ : nat => 9
: nat -> nat
fun (x : nat) (p : x = x) => match p with
| ONE => ONE
end = p
fun (x : nat) (p : x = x) =>
match p in (_ = n) return (n = n) with
| ONE => ONE
end = p
: forall x : nat, x = x -> Prop
fun (x : nat) (p : x = x) => match p with
| 1 => 1
end = p
fun (x : nat) (p : x = x) =>
match p in (_ = n) return (n = n) with
| 1 => 1
end = p
: forall x : nat, x = x -> Prop
bar 0
: nat

0 comments on commit ee912f7

Please sign in to comment.