{"action":"created","comment":{"url":"https://api.github.com/repos/ocaml/ocaml/pulls/comments/197527288","pull_request_review_id":131295332,"id":197527288,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5NzUyNzI4OA==","diff_hunk":"@@ -25,7 +25,9 @@\n type out_ident =\n   | Oide_apply of out_ident * out_ident\n   | Oide_dot of out_ident * string\n-  | Oide_ident of string\n+  | Oide_ident of string ref\n+  (** Beware that identifier names can be renamed on the fly to avoid\n+      name collision between different identifiers, see {!Printtyp} *)\n ","path":"typing/outcometree.mli","position":null,"original_position":8,"commit_id":"f07d73a53fa95d88527f333819548725beae9404","original_commit_id":"c005973551537a89e36124ce4f4d6e7d3b130347","user":{"login":"Octachron","id":7689388,"node_id":"MDQ6VXNlcjc2ODkzODg=","avatar_url":"https://avatars1.githubusercontent.com/u/7689388?v=4","gravatar_id":"","url":"https://api.github.com/users/Octachron","html_url":"https://github.com/Octachron","followers_url":"https://api.github.com/users/Octachron/followers","following_url":"https://api.github.com/users/Octachron/following{/other_user}","gists_url":"https://api.github.com/users/Octachron/gists{/gist_id}","starred_url":"https://api.github.com/users/Octachron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Octachron/subscriptions","organizations_url":"https://api.github.com/users/Octachron/orgs","repos_url":"https://api.github.com/users/Octachron/repos","events_url":"https://api.github.com/users/Octachron/events{/privacy}","received_events_url":"https://api.github.com/users/Octachron/received_events","type":"User","site_admin":false},"body":"A new record type (or maybe an abstract type) does make the intent clearer here indeed. I have pushed a new commit to this effect.","created_at":"2018-06-22T18:12:03Z","updated_at":"2018-06-22T18:12:04Z","html_url":"https://github.com/ocaml/ocaml/pull/1120#discussion_r197527288","pull_request_url":"https://api.github.com/repos/ocaml/ocaml/pulls/1120","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/comments/197527288"},"html":{"href":"https://github.com/ocaml/ocaml/pull/1120#discussion_r197527288"},"pull_request":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/1120"}},"in_reply_to_id":197392771},"pull_request":{"url":"https://api.github.com/repos/ocaml/ocaml/pulls/1120","id":112274782,"node_id":"MDExOlB1bGxSZXF1ZXN0MTEyMjc0Nzgy","html_url":"https://github.com/ocaml/ocaml/pull/1120","diff_url":"https://github.com/ocaml/ocaml/pull/1120.diff","patch_url":"https://github.com/ocaml/ocaml/pull/1120.patch","issue_url":"https://api.github.com/repos/ocaml/ocaml/issues/1120","number":1120,"state":"open","locked":false,"title":"PR#6416 et al.: injective mapping between identifiers and printed names","user":{"login":"Octachron","id":7689388,"node_id":"MDQ6VXNlcjc2ODkzODg=","avatar_url":"https://avatars1.githubusercontent.com/u/7689388?v=4","gravatar_id":"","url":"https://api.github.com/users/Octachron","html_url":"https://github.com/Octachron","followers_url":"https://api.github.com/users/Octachron/followers","following_url":"https://api.github.com/users/Octachron/following{/other_user}","gists_url":"https://api.github.com/users/Octachron/gists{/gist_id}","starred_url":"https://api.github.com/users/Octachron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Octachron/subscriptions","organizations_url":"https://api.github.com/users/Octachron/orgs","repos_url":"https://api.github.com/users/Octachron/repos","events_url":"https://api.github.com/users/Octachron/events{/privacy}","received_events_url":"https://api.github.com/users/Octachron/received_events","type":"User","site_admin":false},"body":"[MPR#6416](https://caml.inria.fr/mantis/view.php?id=6416), [MPR#6634](https://caml.inria.fr/mantis/view.php?id=6634), [MPR#4791](https://caml.inria.fr/mantis/view.php?id=4791) ( see also [MPR#6323](https://caml.inria.fr/mantis/view.php?id=6323), [MPR7402](https://caml.inria.fr/mantis/view.php?id=7402)).\r\n\r\nThis PR aims at getting rid of all instances where different identifiers are printed with the same name in warnings, errors, or inferred signatures; leading to the infamous  \r\n\r\n```OCaml\r\ntype t = A\r\nmodule M: sig type t val x:t end = struct\r\n  type t = B\r\n  let x = A\r\nend;;\r\n…\r\nValues do not match: val x : t is not included in t\r\n\r\n```\r\nand variations.\r\n\r\nSince there are many instances where transforming the above message to use valid types is difficult or impossible, this PR takes the easy road and uses unique but not syntactically valid names to disambiguate between identifiers. For instance, the previous error messages becomes:\r\n\r\n```OCaml\r\nValues do not match: val x : t/1212 is not included in t/1215\r\n```\r\nNote that this naming scheme \"identifier.name/identifier.stamp\" extends the one already used for unification error messages. These unique names are not particularly clear, but they at least convey the fact there was a problem with some type redefinition, and they could be improved later on.\r\n\r\nMoreover, once different identifiers with the same name are disambiguated, it becomes relatively straightforward to detect when a signature printed with `ocamlc -i a.ml` does not corresponds to the real signature of a compilation unit. Therefore, this PR adds a new warning for unprintable signature.\r\n\r\nFor instance, consider the following compilation unit\r\n```OCaml\r\n(* pr6323.ml *)\r\ntype 'a t = B of 'a t list\r\n\r\nlet rec foo f = function\r\n  | B(v)::tl -> B(foo f v)::foo f tl\r\n  | [] -> []\r\n\r\nmodule DT = struct\r\n  type 'a t = {bar : 'a}\r\n  let p t = foo (fun x -> x) t\r\nend\r\n\r\n```\r\nCurrently using `ocamlc -i pr6323.ml` prints\r\n\r\n```OCaml\r\ntype 'a t = B of 'a t list\r\nval foo : 'a -> 'b t list -> 'c t list\r\nmodule DT :\r\nsig type 'a t = { bar : 'a; } val p : 'a t list -> 'b t list end\r\n\r\n```\r\nwhich is the wrong signature, since the true type of `p` is `'a Pr6323.t list -> 'a Pr6323.t list`, not `'a Dt.t list -> 'a DT.t list`.\r\n\r\nWith this PR applied, `ocamlc -w +63 -i pr6323.ml` yields\r\n ```OCaml\r\ntype 'a t = B of 'a t list\r\nval foo : 'a -> 'b t list -> 'c t list\r\nmodule DT :\r\n  sig type 'a t = { bar : 'a; } val p : 'a t/1204 list -> 'b t/1204 list end\r\n```\r\nand emits a warning\r\n```\r\nFile \"pr6323.ml\", line 1:\r\nWarning 63: A signature contained items which could not be printed properly,\r\ndue to name collisions between identifiers.\r\nThe resulting printed signature is not compilable.\r\n```\r\n\r\n## Implementation overview\r\n\r\nOn the implementation side, the first commit in this PR introduces a notion of naming context in `printtyp.ml`. This naming context is used to detect name collision between identifiers when building `outcome` tree. When such a collision is detected, the identifiers is renamed on the fly to a unique name fo the form `identifier.name / identifier.stamp`.\r\n\r\nMoreover, a `Deferred` printing submodule make this naming context easier to use by splitting the rendering of types in two distinct phases:\r\n\r\n * First, build all the concerned `outcome` trees, accumulating information \r\n   in the naming context\r\n\r\n * Then, render the outcome tree, once all name collisions have been resolved\r\n\r\nNote that this mechanism replaces the specific mechanism used previously for unification error messages.\r\n\r\nThe second commit make existing error and warning messages uses this new deferred printing module when appropriate.\r\n\r\nThe third commit introduces the new warning for unprintable signatures, which simply detects if there was any collision event when printing the signature with `ocamlc -i`.\r\n\r\nThe fourth commit extends the naming context collision mechanism to detect name collision with identifiers presents in the printing environment but absent from the current warnings, error messages or signature items. With this extension, most (all?) unprintable signature should be detected.\r\n\r\n## More examples:\r\n\r\n\r\n* signature mismatch [[1]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/typing-misc/pr6416.ml), [[2]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/typing-misc/pr6634.ml)\r\n* ambiguous type [[1]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/typing-warnings/ambiguous_guarded_disjunction.ml.reference)\r\n* printed signature [[1]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/printed-signatures/pr4791.reference), [[2]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/printed-signatures/pr7402.reference), [[3]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/printed-signatures/pervasives_leitmotiv.reference), [[4]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/printed-signatures/pr6323.reference),\r\n* unification error [[1]](https://github.com/Octachron/ocaml/blob/deferred_ident_names/testsuite/tests/typing-misc/unique_names_in_unification.ml )","created_at":"2017-03-23T16:02:46Z","updated_at":"2018-06-22T18:12:04Z","closed_at":null,"merged_at":null,"merge_commit_sha":"fe6d0bfe9e02b5454d9e2ed667389a180ab085c3","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/ocaml/ocaml/pulls/1120/commits","review_comments_url":"https://api.github.com/repos/ocaml/ocaml/pulls/1120/comments","review_comment_url":"https://api.github.com/repos/ocaml/ocaml/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ocaml/ocaml/issues/1120/comments","statuses_url":"https://api.github.com/repos/ocaml/ocaml/statuses/f07d73a53fa95d88527f333819548725beae9404","head":{"label":"Octachron:deferred_ident_names","ref":"deferred_ident_names","sha":"f07d73a53fa95d88527f333819548725beae9404","user":{"login":"Octachron","id":7689388,"node_id":"MDQ6VXNlcjc2ODkzODg=","avatar_url":"https://avatars1.githubusercontent.com/u/7689388?v=4","gravatar_id":"","url":"https://api.github.com/users/Octachron","html_url":"https://github.com/Octachron","followers_url":"https://api.github.com/users/Octachron/followers","following_url":"https://api.github.com/users/Octachron/following{/other_user}","gists_url":"https://api.github.com/users/Octachron/gists{/gist_id}","starred_url":"https://api.github.com/users/Octachron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Octachron/subscriptions","organizations_url":"https://api.github.com/users/Octachron/orgs","repos_url":"https://api.github.com/users/Octachron/repos","events_url":"https://api.github.com/users/Octachron/events{/privacy}","received_events_url":"https://api.github.com/users/Octachron/received_events","type":"User","site_admin":false},"repo":{"id":20154083,"node_id":"MDEwOlJlcG9zaXRvcnkyMDE1NDA4Mw==","name":"ocaml","full_name":"Octachron/ocaml","owner":{"login":"Octachron","id":7689388,"node_id":"MDQ6VXNlcjc2ODkzODg=","avatar_url":"https://avatars1.githubusercontent.com/u/7689388?v=4","gravatar_id":"","url":"https://api.github.com/users/Octachron","html_url":"https://github.com/Octachron","followers_url":"https://api.github.com/users/Octachron/followers","following_url":"https://api.github.com/users/Octachron/following{/other_user}","gists_url":"https://api.github.com/users/Octachron/gists{/gist_id}","starred_url":"https://api.github.com/users/Octachron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Octachron/subscriptions","organizations_url":"https://api.github.com/users/Octachron/orgs","repos_url":"https://api.github.com/users/Octachron/repos","events_url":"https://api.github.com/users/Octachron/events{/privacy}","received_events_url":"https://api.github.com/users/Octachron/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Octachron/ocaml","description":"Read-only mirror of INRIA SVN","fork":true,"url":"https://api.github.com/repos/Octachron/ocaml","forks_url":"https://api.github.com/repos/Octachron/ocaml/forks","keys_url":"https://api.github.com/repos/Octachron/ocaml/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Octachron/ocaml/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Octachron/ocaml/teams","hooks_url":"https://api.github.com/repos/Octachron/ocaml/hooks","issue_events_url":"https://api.github.com/repos/Octachron/ocaml/issues/events{/number}","events_url":"https://api.github.com/repos/Octachron/ocaml/events","assignees_url":"https://api.github.com/repos/Octachron/ocaml/assignees{/user}","branches_url":"https://api.github.com/repos/Octachron/ocaml/branches{/branch}","tags_url":"https://api.github.com/repos/Octachron/ocaml/tags","blobs_url":"https://api.github.com/repos/Octachron/ocaml/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Octachron/ocaml/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Octachron/ocaml/git/refs{/sha}","trees_url":"https://api.github.com/repos/Octachron/ocaml/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Octachron/ocaml/statuses/{sha}","languages_url":"https://api.github.com/repos/Octachron/ocaml/languages","stargazers_url":"https://api.github.com/repos/Octachron/ocaml/stargazers","contributors_url":"https://api.github.com/repos/Octachron/ocaml/contributors","subscribers_url":"https://api.github.com/repos/Octachron/ocaml/subscribers","subscription_url":"https://api.github.com/repos/Octachron/ocaml/subscription","commits_url":"https://api.github.com/repos/Octachron/ocaml/commits{/sha}","git_commits_url":"https://api.github.com/repos/Octachron/ocaml/git/commits{/sha}","comments_url":"https://api.github.com/repos/Octachron/ocaml/comments{/number}","issue_comment_url":"https://api.github.com/repos/Octachron/ocaml/issues/comments{/number}","contents_url":"https://api.github.com/repos/Octachron/ocaml/contents/{+path}","compare_url":"https://api.github.com/repos/Octachron/ocaml/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Octachron/ocaml/merges","archive_url":"https://api.github.com/repos/Octachron/ocaml/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Octachron/ocaml/downloads","issues_url":"https://api.github.com/repos/Octachron/ocaml/issues{/number}","pulls_url":"https://api.github.com/repos/Octachron/ocaml/pulls{/number}","milestones_url":"https://api.github.com/repos/Octachron/ocaml/milestones{/number}","notifications_url":"https://api.github.com/repos/Octachron/ocaml/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Octachron/ocaml/labels{/name}","releases_url":"https://api.github.com/repos/Octachron/ocaml/releases{/id}","deployments_url":"https://api.github.com/repos/Octachron/ocaml/deployments","created_at":"2014-05-25T12:35:17Z","updated_at":"2018-05-03T17:58:01Z","pushed_at":"2018-06-22T18:10:47Z","git_url":"git://github.com/Octachron/ocaml.git","ssh_url":"git@github.com:Octachron/ocaml.git","clone_url":"https://github.com/Octachron/ocaml.git","svn_url":"https://github.com/Octachron/ocaml","homepage":null,"size":182842,"stargazers_count":0,"watchers_count":0,"language":"OCaml","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"trunk"}},"base":{"label":"ocaml:trunk","ref":"trunk","sha":"ae81070b5158dbbf19b25b469d27985d3c66ad91","user":{"login":"ocaml","id":1841483,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE4NDE0ODM=","avatar_url":"https://avatars3.githubusercontent.com/u/1841483?v=4","gravatar_id":"","url":"https://api.github.com/users/ocaml","html_url":"https://github.com/ocaml","followers_url":"https://api.github.com/users/ocaml/followers","following_url":"https://api.github.com/users/ocaml/following{/other_user}","gists_url":"https://api.github.com/users/ocaml/gists{/gist_id}","starred_url":"https://api.github.com/users/ocaml/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ocaml/subscriptions","organizations_url":"https://api.github.com/users/ocaml/orgs","repos_url":"https://api.github.com/users/ocaml/repos","events_url":"https://api.github.com/users/ocaml/events{/privacy}","received_events_url":"https://api.github.com/users/ocaml/received_events","type":"Organization","site_admin":false},"repo":{"id":6786166,"node_id":"MDEwOlJlcG9zaXRvcnk2Nzg2MTY2","name":"ocaml","full_name":"ocaml/ocaml","owner":{"login":"ocaml","id":1841483,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE4NDE0ODM=","avatar_url":"https://avatars3.githubusercontent.com/u/1841483?v=4","gravatar_id":"","url":"https://api.github.com/users/ocaml","html_url":"https://github.com/ocaml","followers_url":"https://api.github.com/users/ocaml/followers","following_url":"https://api.github.com/users/ocaml/following{/other_user}","gists_url":"https://api.github.com/users/ocaml/gists{/gist_id}","starred_url":"https://api.github.com/users/ocaml/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ocaml/subscriptions","organizations_url":"https://api.github.com/users/ocaml/orgs","repos_url":"https://api.github.com/users/ocaml/repos","events_url":"https://api.github.com/users/ocaml/events{/privacy}","received_events_url":"https://api.github.com/users/ocaml/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ocaml/ocaml","description":"The core OCaml system: compilers, runtime system, base libraries","fork":false,"url":"https://api.github.com/repos/ocaml/ocaml","forks_url":"https://api.github.com/repos/ocaml/ocaml/forks","keys_url":"https://api.github.com/repos/ocaml/ocaml/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ocaml/ocaml/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ocaml/ocaml/teams","hooks_url":"https://api.github.com/repos/ocaml/ocaml/hooks","issue_events_url":"https://api.github.com/repos/ocaml/ocaml/issues/events{/number}","events_url":"https://api.github.com/repos/ocaml/ocaml/events","assignees_url":"https://api.github.com/repos/ocaml/ocaml/assignees{/user}","branches_url":"https://api.github.com/repos/ocaml/ocaml/branches{/branch}","tags_url":"https://api.github.com/repos/ocaml/ocaml/tags","blobs_url":"https://api.github.com/repos/ocaml/ocaml/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ocaml/ocaml/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ocaml/ocaml/git/refs{/sha}","trees_url":"https://api.github.com/repos/ocaml/ocaml/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ocaml/ocaml/statuses/{sha}","languages_url":"https://api.github.com/repos/ocaml/ocaml/languages","stargazers_url":"https://api.github.com/repos/ocaml/ocaml/stargazers","contributors_url":"https://api.github.com/repos/ocaml/ocaml/contributors","subscribers_url":"https://api.github.com/repos/ocaml/ocaml/subscribers","subscription_url":"https://api.github.com/repos/ocaml/ocaml/subscription","commits_url":"https://api.github.com/repos/ocaml/ocaml/commits{/sha}","git_commits_url":"https://api.github.com/repos/ocaml/ocaml/git/commits{/sha}","comments_url":"https://api.github.com/repos/ocaml/ocaml/comments{/number}","issue_comment_url":"https://api.github.com/repos/ocaml/ocaml/issues/comments{/number}","contents_url":"https://api.github.com/repos/ocaml/ocaml/contents/{+path}","compare_url":"https://api.github.com/repos/ocaml/ocaml/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ocaml/ocaml/merges","archive_url":"https://api.github.com/repos/ocaml/ocaml/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ocaml/ocaml/downloads","issues_url":"https://api.github.com/repos/ocaml/ocaml/issues{/number}","pulls_url":"https://api.github.com/repos/ocaml/ocaml/pulls{/number}","milestones_url":"https://api.github.com/repos/ocaml/ocaml/milestones{/number}","notifications_url":"https://api.github.com/repos/ocaml/ocaml/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ocaml/ocaml/labels{/name}","releases_url":"https://api.github.com/repos/ocaml/ocaml/releases{/id}","deployments_url":"https://api.github.com/repos/ocaml/ocaml/deployments","created_at":"2012-11-20T22:18:22Z","updated_at":"2018-06-22T07:37:20Z","pushed_at":"2018-06-22T18:10:49Z","git_url":"git://github.com/ocaml/ocaml.git","ssh_url":"git@github.com:ocaml/ocaml.git","clone_url":"https://github.com/ocaml/ocaml.git","svn_url":"https://github.com/ocaml/ocaml","homepage":"","size":172042,"stargazers_count":1858,"watchers_count":1858,"language":"OCaml","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":465,"mirror_url":null,"archived":false,"open_issues_count":157,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":465,"open_issues":157,"watchers":1858,"default_branch":"trunk"}},"_links":{"self":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/1120"},"html":{"href":"https://github.com/ocaml/ocaml/pull/1120"},"issue":{"href":"https://api.github.com/repos/ocaml/ocaml/issues/1120"},"comments":{"href":"https://api.github.com/repos/ocaml/ocaml/issues/1120/comments"},"review_comments":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/1120/comments"},"review_comment":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/1120/commits"},"statuses":{"href":"https://api.github.com/repos/ocaml/ocaml/statuses/f07d73a53fa95d88527f333819548725beae9404"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"Octachron"}}