Merge PR #10336: Improve the ambiguous paths warning to indicate which path is ambiguous with new one

Reviewed-by: SkySkimmer

4 files changed, 29 insertions, 13 deletions

diff --git a/doc/changelog/07-commands-and-options/10336-ambiguous-paths.rst b/doc/changelog/07-commands-and-options/10336-ambiguous-paths.rst
new file mode 100644
index 0000000000..151c400b2c
--- /dev/null
+++ b/doc/changelog/07-commands-and-options/10336-ambiguous-paths.rst
@@ -0,0 +1,5 @@
+- Improve the ambiguous paths warning to indicate which path is ambiguous with
+  new one
+  (`#10336 <https://github.com/coq/coq/pull/10336>`_,
+  closes `#3219 <https://github.com/coq/coq/issues/3219>`_,
+  by Kazuhiko Sakaguchi).
diff --git a/doc/sphinx/addendum/implicit-coercions.rst b/doc/sphinx/addendum/implicit-coercions.rst
index d5523e8561..7fee62179b 100644
--- a/doc/sphinx/addendum/implicit-coercions.rst
+++ b/doc/sphinx/addendum/implicit-coercions.rst
@@ -145,19 +145,25 @@ Declaring Coercions
   .. exn:: Cannot recognize @class as a source class of @qualid.
      :undocumented:
 
-  .. exn:: @qualid does not respect the uniform inheritance condition.
+  .. warn:: @qualid does not respect the uniform inheritance condition.
      :undocumented:
 
   .. exn:: Found target class ... instead of ...
      :undocumented:
 
-  .. warn:: Ambiguous path.
+  .. warn:: New coercion path ... is ambiguous with existing ...
 
-     When the coercion :token:`qualid` is added to the inheritance graph,
-     invalid coercion paths are ignored. The :cmd:`Coercion` command tries to check
-     that they are convertible with existing ones on the same classes.
-     The paths for which this check fails are displayed by a warning in the form
-     :g:`[f₁;..;fₙ] : C >-> D`.
+     When the coercion :token:`qualid` is added to the inheritance graph, new
+     coercion paths which have the same classes as existing ones are ignored.
+     The :cmd:`Coercion` command tries to check the convertibility of new ones and
+     existing ones. The paths for which this check fails are displayed by a warning
+     in the form :g:`[f₁;..;fₙ] : C >-> D`.
+
+     The convertibility checking procedure for coercion paths is complete for
+     paths consisting of coercions satisfying the uniform inheritance condition,
+     but some coercion paths could be reported as ambiguous even if they are
+     convertible with existing ones when they have coercions that don't satisfy
+     the uniform inheritance condition.
 
   .. cmdv:: Local Coercion @qualid : @class >-> @class
 
diff --git a/pretyping/classops.ml b/pretyping/classops.ml
index f5fffc4c1c..afb546b2d2 100644
--- a/pretyping/classops.ml
+++ b/pretyping/classops.ml
@@ -313,7 +313,9 @@ let compare_path p q = !path_comparator p q
 
 let warn_ambiguous_path =
   CWarnings.create ~name:"ambiguous-paths" ~category:"typechecker"
-    (fun l -> strbrk"Ambiguous paths: " ++ prlist_with_sep fnl print_path l)
+    (fun l -> prlist_with_sep fnl (fun (c,p,q) ->
+         str"New coercion path " ++ print_path (c,p) ++
+         str" is ambiguous with existing " ++ print_path (c, q) ++ str".") l)
 
 (* add_coercion_in_graph : coe_index * cl_index * cl_index -> unit
                          coercion,source,target *)
@@ -330,13 +332,13 @@ let different_class_params env i =
 let add_coercion_in_graph env sigma (ic,source,target) =
   let old_inheritance_graph = !inheritance_graph in
   let ambig_paths =
-    (ref [] : ((cl_index * cl_index) * inheritance_path) list ref) in
+    (ref [] : ((cl_index * cl_index) * inheritance_path * inheritance_path) list ref) in
   let try_add_new_path (i,j as ij) p =
     if not (Bijint.Index.equal i j) || different_class_params env i then
       match lookup_path_between_class ij with
       | q ->
         if not (compare_path env sigma p q) then
-          ambig_paths := (ij,p)::!ambig_paths;
+          ambig_paths := (ij,p,q)::!ambig_paths;
         false
       | exception Not_found -> (add_new_path ij p; true)
     else
diff --git a/test-suite/output/relaxed_ambiguous_paths.out b/test-suite/output/relaxed_ambiguous_paths.out
index 2a7ce806d7..dc793598a9 100644
--- a/test-suite/output/relaxed_ambiguous_paths.out
+++ b/test-suite/output/relaxed_ambiguous_paths.out
@@ -1,5 +1,7 @@
 File "stdin", line 10, characters 0-28:
-Warning: Ambiguous paths: [ac; cd] : A >-> D [ambiguous-paths,typechecker]
+Warning:
+New coercion path [ac; cd] : A >-> D is ambiguous with existing 
+[ab; bd] : A >-> D. [ambiguous-paths,typechecker]
 [ab] : A >-> B
 [ab; bd] : A >-> D
 [ac] : A >-> C
@@ -20,8 +22,9 @@ Warning: Ambiguous paths: [ac; cd] : A >-> D [ambiguous-paths,typechecker]
 [D_B] : D >-> B
 [D_C] : D >-> C
 File "stdin", line 103, characters 0-86:
-Warning: Ambiguous paths: [D_C; C_A'] : D >-> A'
-[ambiguous-paths,typechecker]
+Warning:
+New coercion path [D_C; C_A'] : D >-> A' is ambiguous with existing 
+[D_B; B_A'] : D >-> A'. [ambiguous-paths,typechecker]
 [A'_A] : A' >-> A
 [B_A'] : B >-> A'
 [B_A'; A'_A] : B >-> A