Skip to content

Initial implementation of constructor tearoff support #2763

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

Merged
merged 4 commits into from
Aug 25, 2021

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented Aug 24, 2021

Beginning of work for #2655. This is just enough so that if someone switches on constructor tearoffs, dartdoc is not likely to instantly let loose the magic smoke. So it does not support any of the new syntax and still requires users to use the old syntax to refer to default/unnamed constructors.

@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Aug 24, 2021

@Deprecated(
// TODO(jcollins-g): This, in retrospect, seems like a bad idea.
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, why is that? Unnamed constructors and "default" constructors are different things (the latter being a constructor which is implicitly created when a class features no constructors).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly because it is confusing and the two ideas are used interchangeably here. I'll try and clarify.

Copy link
Member

Choose a reason for hiding this comment

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

Ah interesting. Yeah we'd have to see if it is important to distinguish. For example:

class C {
  C(int i);
}

has a constructor whose isUnnamedConstructor is true, but it is not a default constructor (it cannot be implicitly called by any subclass's constructor).

typedef Bt = B;
typedef Ct = C;

/// [Dt.new] should be a thing.
Copy link
Member

Choose a reason for hiding this comment

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

Typo, I think: [D.new]

But this is interesting. It is illegal to tearoff D.new, since it is abstract (it cannot be directly called). But folk may still want to refer to it. Because of this, maybe [D.new] should be left as a PrefixedIdentifier, rather than a ConstructorReference, in the analyzer's AST...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both are intended to work, I'll add the other.

As for the AST implications, I'm guessing it might be a good idea there, yes, for comment reference resolution as well as possibly dealing with erroneous code more cleanly? I don't know the extent to which this is important for those cases.

@jcollins-g jcollins-g force-pushed the constructor-tearoffs-smoke branch from c1d39f6 to aec4729 Compare August 24, 2021 23:25
@jcollins-g jcollins-g requested a review from srawlins August 24, 2021 23:32
@jcollins-g
Copy link
Contributor Author

OK, this is now legit ready for review. It includes migrating the older experiments into the mainline test suite so that we can have experiments that only work in 2.15 without breaking our beta/stable testing.

@jcollins-g jcollins-g merged commit 587a2e2 into dart-lang:master Aug 25, 2021
@jcollins-g jcollins-g deleted the constructor-tearoffs-smoke branch August 25, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants