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

Scope for default values in a primary constructor #3170

Open
munificent opened this issue Jun 28, 2023 · 2 comments
Open

Scope for default values in a primary constructor #3170

munificent opened this issue Jun 28, 2023 · 2 comments
Labels
brevity A feature whose purpose is to enable concise syntax, typically expressible already in a longer form primary-constructors

Comments

@munificent
Copy link
Member

The primary constructors proposal (#3023) defines the scope that the default values are evaluated in as:

The current scope for the default values in the primary constructor is the enclosing library scope. This means that a naive copy/paste operation on the source code could change the meaning of the default value.

Then it goes on to define a careful renaming to ensure that the desugared constructor inside the class body doesn't accidentally refer to an identified that is shadowed by a class member.

Is there a reason for this choice? Given that the primary constructor does end up being a generative constructor on the class, and is syntactically very similar to a constructor declaration, it seems most intuitive to me for it to have the same default value scope as other constructors do: the class scope.

I acknowledge that it's a little weird for a primary constructor's default values to be able to refer to class members when it textually appears before the class's {. But that doesn't seem any weirder to me than:

   T foo<T>();
// ^--- Appears before `<T>`.

I think having the same scope will also make it easier for users and automated refactoring tooling to convert primary constructors to body constructors and vice versa.

@lrhn
Copy link
Member

lrhn commented Jun 28, 2023

I can see the arugument that lexical scoping consistency suggests that code outside of the class body {...} block should not see declarations inside it.

But that code is actually introducing a declaration inside that scope.
If I write:

class Gee<T>.bee(int s) {
  static get bee => 42;
  static get T => 42;
  static get s => 42;
}

I have a naming conflict for all of bee, T and s. All three introduce names that are available, and must not be shadowed/conflicted with, inside the class body.

I'm willing to sacrifice needles consistency for conveninece here.
Allowing the class body declaratiosn to be in scope makes it's easier to convert to/from a primary constructor.
Take:

class Foo {
  static const defaultX = 42;
  final int x;
  Foo([this.x = defaultX]);
}

Converting that to a primary constructor requires writing it as:

class Foo([this.x = Foo.defaultX]) { // Had to add `Foo.` here
  static const defaultX = 42;
  final int x;
}

Adding the Foo. is not a big sacrifice, but it is a difference.
And I'll bet that if we have a quick-fix for turning it back into a constructor declaration, it will end up as:

class Foo {
  static const defaultX = 42;
  final int x;
  Foo([this.x = Foo.defaultX]);
}

because nobody will check whether the Foo is no longer necessary. (Why bother, it still works!)

But I also think it's not a big deal if we keep using the outer scope. It only affects default value expressions, there are no other expressions in a primary constructor where scope can matter.
If it avoids someone doing;

const defaultValue = 42;
class Foo([int x = defaultValue]) {
  static const defaultValue = "Foo";
}

and getting confused why defaultValue doesn't resolve the way they expect it to, and others having to write Foo. infront of their constants, it'll still work.

Basically, it's a tradeoff, and I can't tell which way it should go.

(Ceterum, I also think declarations from inside a do {body} while (e)'s body should be visible in the e expression.
Because it's bloody annoying that they're not, and you have to declare a variable outside just to test it in the loop condition. This needless consistency is just getting in the way of better code.)

@eernstg
Copy link
Member

eernstg commented Jun 29, 2023

I have no strong opinions here, I just thought it was really weird to allow a lookup to look into a {...}. ;-)

@eernstg eernstg added the brevity A feature whose purpose is to enable concise syntax, typically expressible already in a longer form label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brevity A feature whose purpose is to enable concise syntax, typically expressible already in a longer form primary-constructors
Projects
None yet
Development

No branches or pull requests

3 participants