Skip to content

Conversation

leafpetersen
Copy link
Member

@leafpetersen leafpetersen commented Mar 31, 2020

This changes the specification of NNBD_TOP_MERGE to resolve any conflicting top types to Object?, per discussion here.

Initial set of tests here, more to follow.
[Edit: more tests here]

cc @eernstg @munificent @lrhn

- `NNBD_TOP_MERGE(dynamic, dynamic) = dynamic`
- `NNBD_TOP_MERGE(void, void) = void`
- `NNBD_TOP_MERGE(Object?, void) = void`
- `NNBD_TOP_MERGE(Object?, void) = Object?`
Copy link
Member

@lrhn lrhn Mar 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You generally use precedence, so would it be enough to write

  • NNBD_TOP_MERGE(x, y) = Object?, x, y in {void, Object?, dynamic}
    (perhaps with an "otherwise" attached, or an x != y).

Not necessary, just perhaps easier to understand.

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I don't see any reason to use a different style that the "list all cases" style we have now.

@leafpetersen
Copy link
Member Author

Tests for member signatures out for review here

@leafpetersen leafpetersen merged commit 9c63d7c into master Apr 8, 2020
@leafpetersen leafpetersen deleted the top_merge branch April 8, 2020 20:25
@leafpetersen leafpetersen added the nnbd NNBD related issues label Apr 8, 2020
dart-bot pushed a commit to dart-lang/sdk that referenced this pull request Apr 8, 2020
Tests for member signature resolution as specified in this change:
dart-lang/language#904

Change-Id: I7ee81dd1927b8f9419082f9bd2cbec14f0a5c0f0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/142802
Reviewed-by: Erik Ernst <eernst@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes nnbd NNBD related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants