Fix #9640: regression introduced by #9623#9642
Conversation
|
I see how this fixes the problem, thanks! I think the code would be easier to read if it was scructured as follows. (I thought of trying that during #9623 but I didn't want to change the code too much. I now regret not doing it, as I think it could have revealed the issue you are now fixing.) let rec merge_in_sig outer_env namelist sg =
let sig_env = Env.add_signature sg outer_env in
merge_path sig_env namelist sg
and merge_path sig_env namelist sig_items =
match (namelist, sig_items) with
| ([name], sig_items) ->
merge_name sig_env name None sig_items
| (s :: namelist, (Sig_module(id, _, md, rs, priv) as item) :: rem)
when Ident.name id = s ->
let ((path, _path_loc, tcstr), new_sig_items) =
merge_in_sig (extract_sig sig_env loc md.md_type) namelist in
real_ids := path :: !real_ids;
let item =
match md, constr with
| {md_type = Mty_alias _}, (Pwith_module _ | Pwith_type _) ->
item
| _ ->
let newmd = { md with md_type = Mty_signature new_sig_items } in
Sig_module(id, Mp_present, newmd, rs, priv)
in
(path, lid, tcstr),
item :: rem
| (_, []) ->
raise(Error(loc, sig_env, With_no_component lid.txt))
| (_, item :: rem) ->
let (cstr, items) = merge_path sig_env namelist rem in
cstr, item :: items
and merge_name sig_env name row_id sig_items =
match sig_items with
| (* all the other cases *)
| [] ->
raise(Error(loc, sig_env, With_no_component lid.txt))
| item :: rem ->
let cstr, items = merge_name sig_env name row_id rem in
cstr, item :: items
in |
|
I did the simpler part of your change (factorize signature processing and handling of submodules). The other part seems to duplicate more than it factorizes. |
|
I also did one last factorisation for |
gasche
left a comment
There was a problem hiding this comment.
The factorization are good, and I believe that the fix is correct.
|
Merged, and cherry-picked in 4.11 ( ef2d35e ). |
In
merge_constraint, #9623 was precomputing a commonsig_envfor the whole signature, but forgot to do so recursively.This PR fixes #9640.