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

Consider removing support for null-aware explicit extension invocations #4201

Open
stereotype441 opened this issue Dec 10, 2024 · 2 comments
Labels
extension-methods null-aware-expressions Issues proposing new expressions to manage null

Comments

@stereotype441
Copy link
Member

@eernstg points out in dart-lang/sdk#59654 (comment) that according to the spec, a null-aware explicit extension invocation such as E(c)?.g... is a compile-time error.

As far as I can tell, both the analyzer and the CFE have supported this behavior since the inception of the "extension methods" feature. Here is a link to the text from the original feature specification suggesting that this was intended to be supported: https://github.com/dart-lang/language/blob/main/accepted/2.7/static-extension-methods/feature-specification.md#null-aware-member-invocations

I'm moving discussion about this from dart-lang/sdk#59654 to here, since (a) dart-lang/sdk#59654 is about a specific problem with flow analysis and the analyzer, and (b) the change being proposed is a change to the behavior of both implementations, and so even if we decide to make this change in order to bring the implementations in line with existing specification text, I think we should treat it as a language change.

@stereotype441
Copy link
Member Author

@eernstg further commented:

The situation might be a bit underspecified, but we do have some pieces.

As you indicate, the document language/accepted/2.7/static-extension-methods
/feature-specification.md is a background document at this time, so we should rely on the language specification where this feature is specified today. Of course, we might still detect a mistake and fix it in the language specification.

However, the rule which makes E(c) a compile time error occurs in the feature specification as well, here:

It is a compile-time error if the corresponding class constructor invocation would be a compile-time error.

This doesn't prevent every null-aware explicit extension member invocation from taking place, because we could have an instantiated on-type which is nullable:

class C {}

extension E2 on C? {
  String foo() => 'Foo!';
}

void test(C? c) {
  print(E2(c)?.foo());
}

void main() => test(null);

However, the implicit form is more flexible, based on the language specification, as described here:

\subsection{Member Invocations}
. So c?.g(c.f()) should be OK in the original example.

It could be considered to be an inconsistency that E(c)?.g... is a compile-time error in the original example, but E2(c)?.foo() is not an error, but it does make sense to me that we simply insist on E(c) being subject to a type check based on the instantiated on-type. It is somewhat "magical" if the ? on the outside of E(c) can eliminate an error inside E(c).

@lrhn
Copy link
Member

lrhn commented Dec 10, 2024

It is not a compile-time error, and it's working as intended and specified.

As I remember it (vaguely, but it's coming back), this was a deliberate way to allow the explicit invocation to represent the same thing as the implicit extension invocation c?.foo.

Without this rule, there is no (easy) way to rewrite c?.foo into an explicit invocation in case of name conflicts. Or just as a way to specify what it means in terms of a single explicit invocation.

The meaning of E(c)?.foo as a whole (a null-aware explicit extension invocation) is to evaluate c to a value, if that value is null then the entire expression evaluates to null, otherwise invoke the foo method of E on the non-null value. With Null Safety the static type that E is attempted applied to is NonNull(S), where S is the static type of c, not S itself.
So equivalent to let $tmp = c in $tmp == null ? null : E($tmp).foo, where the latter $tmp is promoted to NonNull(S).

The text predates null-safety, so one can't read the main text and assume that it talks about nullable types. The referenced text is for pre-null-safety types where the corresponding constructor invocation E(c) would not be an error.

With null safety, the corresponding constructor invocation would be on the promoted value, and should only be an error if E(c!).foo would be a compile-time error.
From the spec:

(With NNBD, the type of e1 is promoted to non-null before inferring the on type of the extension application, just as for the implicit invocation e1?.…, and the result type becomes nullable if it isn't already.)

The spec was ready for null safety with the modifications needed in separate sections, and the implementations are implementing the intended NNBD-behavior correctly.

It's all good.

(If we had a null-safety aware spec, it should be saying the same thing, otherwise that's a spec bug).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-methods null-aware-expressions Issues proposing new expressions to manage null
Projects
None yet
Development

No branches or pull requests

2 participants