{"action":"opened","issue":{"url":"https://api.github.com/repos/dart-lang/sdk/issues/38698","repository_url":"https://api.github.com/repos/dart-lang/sdk","labels_url":"https://api.github.com/repos/dart-lang/sdk/issues/38698/labels{/name}","comments_url":"https://api.github.com/repos/dart-lang/sdk/issues/38698/comments","events_url":"https://api.github.com/repos/dart-lang/sdk/issues/38698/events","html_url":"https://github.com/dart-lang/sdk/issues/38698","id":501760206,"node_id":"MDU6SXNzdWU1MDE3NjAyMDY=","number":38698,"title":"NNBD migration tool incorrectly considers parameters as postdominated","user":{"login":"MichaelRFairhurst","id":1627771,"node_id":"MDQ6VXNlcjE2Mjc3NzE=","avatar_url":"https://avatars1.githubusercontent.com/u/1627771?v=4","gravatar_id":"","url":"https://api.github.com/users/MichaelRFairhurst","html_url":"https://github.com/MichaelRFairhurst","followers_url":"https://api.github.com/users/MichaelRFairhurst/followers","following_url":"https://api.github.com/users/MichaelRFairhurst/following{/other_user}","gists_url":"https://api.github.com/users/MichaelRFairhurst/gists{/gist_id}","starred_url":"https://api.github.com/users/MichaelRFairhurst/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MichaelRFairhurst/subscriptions","organizations_url":"https://api.github.com/users/MichaelRFairhurst/orgs","repos_url":"https://api.github.com/users/MichaelRFairhurst/repos","events_url":"https://api.github.com/users/MichaelRFairhurst/events{/privacy}","received_events_url":"https://api.github.com/users/MichaelRFairhurst/received_events","type":"User","site_admin":false},"labels":[{"id":1534761059,"node_id":"MDU6TGFiZWwxNTM0NzYxMDU5","url":"https://api.github.com/repos/dart-lang/sdk/labels/NNBD","name":"NNBD","color":"fef2c0","default":false},{"id":1364724953,"node_id":"MDU6TGFiZWwxMzY0NzI0OTUz","url":"https://api.github.com/repos/dart-lang/sdk/labels/analyzer-nnbd-migration","name":"analyzer-nnbd-migration","color":"d4c5f9","default":false},{"id":219928186,"node_id":"MDU6TGFiZWwyMTk5MjgxODY=","url":"https://api.github.com/repos/dart-lang/sdk/labels/area-analyzer","name":"area-analyzer","color":"c7def8","default":false},{"id":334529063,"node_id":"MDU6TGFiZWwzMzQ1MjkwNjM=","url":"https://api.github.com/repos/dart-lang/sdk/labels/type-bug","name":"type-bug","color":"ededed","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2019-10-02T22:35:04Z","updated_at":"2019-10-02T22:35:04Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"This is a pathological attempt (and its successful) to break our goal of preservation of behavior in the current design of the migration tool.\r\n\r\n```\r\nclass A {\r\n  f(int x) {\r\n    x + 1;\r\n  }\r\n}\r\n\r\nclass B {\r\n  f(int x) {\r\n    print(x);\r\n  }\r\n}\r\n\r\nvoid g(A a) {\r\n  a.f(null);\r\n}\r\n\r\nvoid main() {\r\n  g(B());\r\n}\r\n```\r\n\r\n*Which nullabilities get changed*: `A.f` is made non-null, because its parameter is postdominated by a non-null assertion through calling the `+` parameter on it. This causes the call `a.f(null)` in `g` to get a `!` on the `null` literal.\r\n\r\n*The old behavior*: This program prints \"null\".\r\n\r\n*The new behavior\": The migrated program will crash.\r\n\r\nThe error here is that hard edges are added when we have a post-dominating non-null intent on a type/element. In this case, we have mistakenly drawn the conclusion that the code `x + 1` postdominates the parameter `x`. However, as shown by `main` and `g`, this is not actually the case.\r\n\r\n`x + 1` only postdominates _the local binding of the parameter_ `x`, and does not postdominate references to the _parameter type in the signature of `A.f`_.\r\n\r\n*Some options going forward*:\r\n\r\n- don't draw hard edges to parameters (probably *not* what we want?)\r\n- make null literals have something harder than a hard edge, so that the `null` literal wins over the \"postdominating\" use of x. This does not fully solve the problem, though, as the same problem holds for any nullable var which may hold the value null.\r\n- make hard edge propagation stop when it sees a hard null propagation. This is like the prior option but could handle postdominated usages of nullable parameters.\r\n- make hard edges for parameters when no overrides of that method exists (would require an additional analysis pass and miss downstream overrides)\r\n- add some kind of edge to the graph solver where it waits to see that *all* usages of *all* methods (ie, base and overrides) for a given parameter are postdominated by a non-null usage before propagating the hard edge backwards to callers\r\n\r\nSurely there are others that I haven't thought of.\r\n\r\n"}}
{"actor":{"display_login":"MichaelRFairhurst"}}