Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review design for binding string with nullability off #26199

Closed
jcouv opened this issue Apr 17, 2018 · 3 comments
Closed

Review design for binding string with nullability off #26199

jcouv opened this issue Apr 17, 2018 · 3 comments

Comments

@jcouv
Copy link
Member

jcouv commented Apr 17, 2018

Currently, we bind string as string! when the nullability feature is on, but as string~ (oblivious) when the feature is off (LangVersion=7 for example).

The scenario driving this is a C# 7.0 compilation being referenced in a C# 8.0 compilation. We want symbols from the compilation reference to be re-used, but we want the C# 8.0 compilation to treat them as oblivious.

A few options we discussed so far:

  1. bind symbols differently depending on features being on/off (current design)
  2. wrap symbols from compilation references in some cases, rather than re-use them directly (so a string! from a C# 7.0 compilation reference appears as oblivious to the C# 8.0 compilation)
  3. have the caller be smart (you're given a source symbol, you should check if it comes from a compilation with nullability on/off). (this seems error-prone)
@jcouv
Copy link
Member Author

jcouv commented Apr 17, 2018

Tagging @AlekseyTs @gafter @cston to start discussion.
How strongly do we want to stick to the rule that binding should produce the same result regardless of features that are turned on/off or language version?

@AlekseyTs
Copy link
Contributor

I don't think the current behavior should be treated as a rule violation. The rule is more like: "If you have a syntactic construct that clearly "points" to a new feature, bind it as such even if feature is not allowed and report an appropriate diagnostic". In this case, there is no special syntax indicating that any new feature is used.

@jcouv
Copy link
Member Author

jcouv commented Jul 11, 2018

This has been resolved in LDM today. The flag changes the implicit URTANN/NonNullTypes context, but should not otherwise affect binding or emitting.

@jcouv jcouv closed this as completed Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants