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

Implement constructor tear-off support in analyzer. #46020

Closed
stereotype441 opened this issue May 14, 2021 · 4 comments
Closed

Implement constructor tear-off support in analyzer. #46020

stereotype441 opened this issue May 14, 2021 · 4 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@stereotype441
Copy link
Member

stereotype441 commented May 14, 2021

This issue tracks support of the constructor tearoffs feature. See the enclosing meta-issue for details.

@stereotype441 stereotype441 added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label May 14, 2021
dart-bot pushed a commit that referenced this issue May 15, 2021
Change-Id: Ic82dd28135ccaa2d5b42898642a1d6e1e0868cb0
Bug: #46020
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199680
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot pushed a commit that referenced this issue May 17, 2021
Change-Id: I8e00eb6094733238549ca259e4355032fc8c1cea
Bug: #46020
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199681
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@devoncarew devoncarew added the P2 A bug or feature request we're likely to work on label May 17, 2021
@devoncarew
Copy link
Member

cc @srawlins

@franklinyow - it looks like some constructor tear-off implementation issues are starting to be filed; it might be worth creating the github tracking project now, in order to group the related issues?

@srawlins
Copy link
Member

Hi @stereotype441 I think you mentioned the first step (after design) is to add the AST nodes. Did you say you would do this? For CFE and/or analyzer?

@stereotype441
Copy link
Member Author

@srawlins yep, new AST nodes added in 81c45e4, and visitor changes added in 7584faf. I have been working on parser support, and hope to have a CL out for that today.

dart-bot pushed a commit that referenced this issue May 25, 2021
This CL adds parser support for use of `<typeArguments>` as a
selector.  This allows expressions like `List<int>` (type literal with
type arguments), `f<int>` (function tear-off with type arguments),
`C.m<int>` (static method tear-off with type arguments), `EXPR.m<int>`
(instance method tear-off with type arguments), and `EXPR<int>`
(tear-off of `.call` method with type arguments).

I will add parser support for `.new` as a constructor name in a
follow-up CL.

Change-Id: I157e732276421e8c3fd20c38c67ae9643993bd85
Bug: #46020, #46044.
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197102
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
dart-bot pushed a commit that referenced this issue Jun 1, 2021
…rguments.

Now that we have an AST and visitor support for the FunctionReference
structure (which represents `Expression<TypeArguments>` for various
kinds of expressions), we no longer need to error recover this as a
FunctionExpressionInvocation with synthetic arguments.  The resolver
still doesn't resolve the syntax properly, but that's ok because it's
not permitted in valid code (for now we just treat it as having type
`dynamic`).

Fixes #46150.

Change-Id: I357175cc16bcf2f9027be2e1da66bb6ca70a9400
Bug: #46020
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199682
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot pushed a commit that referenced this issue Jun 2, 2021
The new "constructor tearoffs" feature includes the ability to use the
`new` keyword in a place where an identifier is expected, and that
identifier could denote a constructor name.  We handle this by
replacing the `new` keyword token with an identifier token whose
identifier string is `new` (this should ease the burden on the
implementations, since they are already set up to handle constructor
names that are identifiers).

Since all such situations follow a `.`, and `new` was never previously
allowed after a `.`, the parser treats `new` as an identifier in any
situation where it could possibly refer to a constructor, regardless
of whether the "constructor tearoffs" feature is enabled.  (This
should allow for easier error recovery in the situation where the user
tries to use the feature with a language feature that does not support
it).  It is up to the client to report an error if the feature is
disabled.

In this CL, I've implemented logic for the analyzer to choose whether
to report an error based on whether the feature is enabled.  I've
implemented logic for the CFE to report the error unconditionally.

Bug: #46020, #46044
Change-Id: I36a496688400d2d9f699dd42be4d0ba620cda244
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201961
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
@srawlins srawlins self-assigned this Jul 29, 2021
dart-bot pushed a commit that referenced this issue Aug 1, 2021
This resolver may need to rewrite the AST if the TypeName resolves to be
a function reference or a constructor reference with type-instantiation.

Bug: #46020 #46721
Change-Id: Ie6a9aa3c04d739becc0c902c117a7151f9c1fcf1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208540
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dart-bot pushed a commit that referenced this issue Aug 3, 2021
This adds support for both declaring an unnamed constructor with the
explicit name, "new", and support for invoking an unnamed constructor
as a named constructor named "new".

The parser will report EXPERIMENT_NOT_ENABLED if the experiment is not
enabled, as the parser takes care around the keyword, "new".

Tearoff support will be separate.

Bug: #46020
Change-Id: Iaf3af333dd22337b560aa7f4e5811a4cb38b2a7f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208760
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dart-bot pushed a commit that referenced this issue Aug 3, 2021
Bug: #46020
Change-Id: I5a737b8b227fa205e66650dd40d697e4e3ef0125
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208821
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dart-bot pushed a commit that referenced this issue Aug 9, 2021
Bug: #46020
Change-Id: Ifafea6edb6c9ce7fdf1c1f092ff278738c273bb0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209300
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dart-bot pushed a commit that referenced this issue Aug 10, 2021
From the spec:

> We do not allow dynamic explicit instantiation. If an
expression _e_ has type `dynamic` (or `Never`), then
`e.foo<int>` is a compile-time error for any name `foo`.

Bug: #46020
Change-Id: I041c3fcac77fe8edf64633a8d23dbaa89cf42f77
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209623
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot pushed a commit that referenced this issue Aug 11, 2021
This should handle all cases of constructor reference w/ explicit type
args; there aren't too many cases:

* named and unnamed constructors
* referencing class and referencing type alias
* prefixed class names and not-prefixed
* null-aware access (weird)
* bound on type parameter of class, and on type parameter of alias

error cases:

* cascade
* wrong number of type arguments

Bug: #46020
Change-Id: If257eb561a9ad854709b6e9a7d81faa9d084d6ee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209622
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot pushed a commit that referenced this issue Aug 23, 2021
Today, the parser reads `a<...>.b()` as a constructor call with
explicit type arguments for the class. This could be a method call
on a function tearoff (however unlikely).

Fixes #46721

Bug: #46020
Change-Id: I16681171a7d99adf3fd080a235fcedc46c5fb1ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210780
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot pushed a commit that referenced this issue Sep 1, 2021
…ions

This allows local variables and other expressions, as long as the
static type is a generic function type, to be explicitly type
instantiated.

* Expressions with a non-generic function type cannot be type
  instantiated.
* Expressions with a type of `Function` cannot be type instantiated.
* Expressions with other non-function types cannot be type instantiated.

This results in doubling-up of errors in a few situations. I think these
are generally rare occurrences, and tricky to prevent double reporting,
so I've left them for now.

Bug: #46020,
Change-Id: Iad212fd95773f39f3202480b3fa71f6a28c7698f
dart-lang/language#1812
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211941
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
dart-bot pushed a commit that referenced this issue Sep 14, 2021
#46020

Change-Id: Ib985b70773bb1a4c90742af0742a89e78f84ac98
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/213293
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dart-bot pushed a commit that referenced this issue Sep 15, 2021
This code is a little weird because of how this type instantiation
works:

```
typedef Fn<T> = void Function(T);

var x = Fn<int>.foo;
var y = (Fn<int>).foo;

extension on Type {
  int get foo => 1;
}
```

`x` is illegal under any circumstance, because calling a getter on a
type instantiation can _only_ resolve to a constructor, but function
types do not have constructors. But it's nice to resolve what we can,
and what the user may have meant, and we have to represent
`Fn<int>.foo` _somehow_. So it's a property access on a TypeLiteral.

Add two new codes because it is not correct to say that `foo` is not a
getter on 'Type' because that is beside the point. The issue is that
there is no possible getter on a type-instantiated type literal of
a function type alias (nor method, nor setter).

Add lots of tests, for calling a method, a getter, and a setter on a
function type alias literal. Add tests with prefixes, bounds, too
many and too few args.

Bug: #46020
Change-Id: Icdf17506a64b3382226c5e50786784130d9e3bf9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/213287
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
copybara-service bot pushed a commit that referenced this issue Sep 26, 2021
In particular, check the two cases where a constructor reference or
function reference may appear in which they must be constant
expressions, but are not in constant contexts:

* default parameter value
* field initializer in a class with at least one generative const
  constructor

We also update NodeReplacer to also replace a DefaultParameterElement's
reference to the AST of the default value.

Bug: #46020
Change-Id: Iea92265d41459b9f79317128e96d80de5069abfc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214340
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit that referenced this issue Sep 27, 2021
Additionally, fix instantiate-to-bounds in implicit-type-arg type literals in const expressions.

Bug: #46020
Change-Id: I6357d25a525bfedfcdd68d4a27f63bdd121f5e07
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214420
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit that referenced this issue Sep 29, 2021
Given an expression like `e<T>`, where the static type of `e` is a callable type, treat the expression as if it were a type instantiated tearoff of the `call` method, as in `e.call<T>`.

Bug: #46020
Change-Id: I62fb23677b0df04468dd48532e82ec5ad46fc685
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214869
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit that referenced this issue Oct 1, 2021
…nment

This fix is pretty simple; we have the inferred type available on [node], so we just need to create a new DartObject which uses that type.

Bug: #46020
Change-Id: Ib53ff381b8c9c864f97eb2a4869d3798e028c30f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215003
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit that referenced this issue Oct 6, 2021
…perty access

Bug: #46020
Change-Id: I99bfa6d96b84a41abdd02d41ff6ad7dd5c99789c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215687
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit that referenced this issue Oct 6, 2021
Additionally, I see that _deferred types_ are not constant, and make
great examples of non-const function references, constructor references,
and type literals.

Fixes language/const/constant_type_variable_test

Bug: #46020,
     #47302
Change-Id: I9df3a9eb758d058888f7d374b76756ec1443c8d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215860
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
copybara-service bot pushed a commit that referenced this issue Oct 13, 2021
Bug: #46020

Fixes language/const/instantiated_function_constant_test

Change-Id: I7b9a92a7abca213919d0850dd68c5b2d3d4857f5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/216601
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
@devoncarew devoncarew added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Oct 13, 2021
copybara-service bot pushed a commit that referenced this issue Oct 20, 2021
Fixes half a dozen tests for
#46020

Fixes #47471

Fixes #47366

There are two cases where implicit tear-off conversion should be
performed in desugared code. Because the analyzer does not desugar,
we just make additional checks in assignability:
* for-in statements, where the iterable is an Iterable<C> and C is
  a callable class, and the for-in element is function-typed.
* spread-elements, where the iterable is an Iterable<C> and C is
  a callable class, and the typed literal element type is a
  function type.

This is a breaking change as it involves removing implicit tear-off
conversion from TypeSystem.isAssignableTo.

However, it passes google3 today. :D

Change-Id: I9d852adadc11c92b3f7dbf130965d003c68b2b14
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217157
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
copybara-service bot pushed a commit that referenced this issue Oct 26, 2021
This is a non-breaking change (when analyzing code at language version
2.14 and earlier). If constructor-tearoffs is enabled (language version
2.15), then FunctionReference nodes are inserted, to represent generic
function instantiation. Constant evaluation can then use the
`typeArgumentTypes` to instantiate arbitrary function-typed
expressions.

If constructor-tearoffs is not enabled, generic function instantiation
continues to take place at a SimpleIdentifier, PrefixedIdentifier, or
PropertyAccess only, with constant evaluation using
SimpleIdentifier.tearOffTypeArgumentTypes.

Bug: #46020
Change-Id: Ie370c00c8a2ce7a4791ac9cdb7459a01339a79c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217880
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
copybara-service bot pushed a commit that referenced this issue Oct 26, 2021
This reverts commit 1c8f56f.

Reason for revert: This breaks package:collection at
ListComparableExtensions.binarySearch, at
`comparable ?? compareComparable`:

The argument type 'Function' can't be assigned to the parameter type
'intFunction(E, E)'. #argument_type_not_assignable

Original change's description:
> Implement Generic Function Instantiation via wrapping node
>
> This is a non-breaking change (when analyzing code at language version
> 2.14 and earlier). If constructor-tearoffs is enabled (language version
> 2.15), then FunctionReference nodes are inserted, to represent generic
> function instantiation. Constant evaluation can then use the
> `typeArgumentTypes` to instantiate arbitrary function-typed
> expressions.
>
> If constructor-tearoffs is not enabled, generic function instantiation
> continues to take place at a SimpleIdentifier, PrefixedIdentifier, or
> PropertyAccess only, with constant evaluation using
> SimpleIdentifier.tearOffTypeArgumentTypes.
>
> Bug: #46020
> Change-Id: Ie370c00c8a2ce7a4791ac9cdb7459a01339a79c1
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217880
> Commit-Queue: Samuel Rawlins <srawlins@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

TBR=scheglov@google.com,brianwilkerson@google.com,srawlins@google.com

Change-Id: Ie76cd703e05cbf65fecaa76b72e829f8272b8307
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #46020
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/218089
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit that referenced this issue Oct 27, 2021
This is a re-land for https://dart-review.googlesource.com/c/sdk/+/217880.

The primary difference is the call-sites of
insertGenericFunctionInstantiation. There are more, and many more test
cases.

This is a non-breaking change (when analyzing code at language version
2.14 and earlier). If constructor-tearoffs is enabled (language version
2.15), then FunctionReference nodes are inserted, to represent generic
function instantiation. Constant evaluation can then use the
`typeArgumentTypes` to instantiate arbitrary function-typed
expressions.

If constructor-tearoffs is not enabled, generic function instantiation
continues to take place at a SimpleIdentifier, PrefixedIdentifier, or
PropertyAccess only, with constant evaluation using
SimpleIdentifier.tearOffTypeArgumentTypes.


Bug: #46020
Change-Id: Icf876cb6866bc1030e0cefaaafe221757d5b5639
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/218221
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
@srawlins
Copy link
Member

srawlins commented Nov 1, 2021

Done.

@srawlins srawlins closed this as completed Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

3 participants