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

Make pseudo-privately named actual arguments an error? #4062

Open
eernstg opened this issue Aug 29, 2024 · 9 comments
Open

Make pseudo-privately named actual arguments an error? #4062

eernstg opened this issue Aug 29, 2024 · 9 comments
Labels
question Further information is requested

Comments

@eernstg
Copy link
Member

eernstg commented Aug 29, 2024

Thanks to @sgrekhov for bringing up this situation.

It is currently a compile-time error for a named parameter to have a name whose first character is _.

void f({int _ = 0}) {} // Error.

However, it is not reported as a compile-time error when an actual argument is passed to such (non-existing) parameters:

void main() {
  Function f = () {};
  try {
    f(_pseudoPrivateName: 1); // OK
  } catch (_) {}
}

Should we make it a compile-time error to use actual arguments with such names?

We may introduce the ability to declare a formal parameter whose name starts with _, but we do not (as far as I can see) have any intentions to allow the parameter to have such a name, it's only considered for situations where the formal parameter name will be computed as the 'corresponding public name'. (The corresponding public name is p when the declared name is _p, but it does not exist when the declared name is _ or __ or _1, so we only intend to allow formal parameters with such declared names when other constraints are satisfied as well.)

@dart-lang/language-team, WDYT? Should we report f(_pseudoPrivateName: 1) as a compile-time error, based on the reasoning that it cannot succeed in current programs, and most likely will never succeed even in future Dart code?

@lrhn
Copy link
Member

lrhn commented Aug 29, 2024

Yes. It should be an error if a named parameter or argument has a name starting with an underscore.
(I've always assumed that to be what we had specified.)

@natebosch
Copy link
Member

I doubt this comes up much so I wouldn't make it a high priority, but shifting a runtime error to a static error is a clear improvement if it does come up.

@lrhn
Copy link
Member

lrhn commented Aug 30, 2024

It can only ever become a runtime error in dynamic calls, since no function type has a named parameter with a private name. (I hope. Did we remember to say that function types cannot have private named parameters? I think we did, CFE and analyzer both complain in DartPad.)

I doubt it ever happens in practice, but I also never intended to allow it to begin with. I'd be perfectly happy to steath-nerf the capability and claim it as a bug-fix.

@eernstg
Copy link
Member Author

eernstg commented Aug 30, 2024

Very good, so this issue will target actual arguments as well as function type parameters. The existing error occurs in the section about 'Function Declarations' and it makes sense to require that it's mentioned along with the syntax for the corresponding part of a function type (that is, <parameterTypeList> and non-terminals derived from that).

@lrhn
Copy link
Member

lrhn commented Aug 30, 2024

I think all implementations already implement the rule for function types. They use the same error message as for function declarations, so maybe they just did it in the function type, and checks that the declaration is valid by checking that its type is valid.

@leafpetersen
Copy link
Member

It can only ever become a runtime error in dynamic calls, since no function type has a named parameter with a private name.

This isn't true. The code below currently prints 3.

class Foo {
  noSuchMethod(_) => 3;
  
}
void main() {
  dynamic x = Foo();
  print(x.bar(_nonsense : 3));
}

I'd be fine with making this an error, but I suspect the added user value is zero, so I'm not sure it's really worth the bother.

@lrhn
Copy link
Member

lrhn commented Aug 30, 2024

That doesn't contradict that it can only give a runtime error for a dynamic invocation.
It's just that it can also not give an error, which is slightly surprising, and worrisome.

The miniscule user benefit would be to give an early error if someone writes a private named argument by accident.
Or if or happens in generated code.

The added tooling benefit is that backends can trust there to be no private named arguments, so we don't risk a compiler error instead of a runtime error.
Or incorrect behavior. For example, do we know if the symbol passed to noSuchMetod is really private, or was it created using new Symbol("_nonsense")? That would be valid if there were no private names.
(Checked, changing the line to

  noSuchMethod(i) => i.memberName == #_nonsense;

and DartPad prints false. I don't think that's specified behavior. In fact, I don't think we have specified what happens in this case at all. So we should either disallow, or actually specify the behavior, stating whether the symbol is private or not. Which likely means we need to do both specification, testing and either front-end or backend implementation whether we disallow it or officially allow it.)

@leafpetersen
Copy link
Member

That doesn't contradict that it can only give a runtime error for a dynamic invocation.

Sorry, I misunderstood you to be saying that every such dynamic call must result in a runtime error (and hence that no normally terminating program could have such code in it).

@gryznar
Copy link

gryznar commented Sep 27, 2024

IMHO compiler should not threat formal named parameters, starting with _ as an error. Sometimes it is very beneficial to threat this like implementation detail. Privacy rules in this case should be applied (not available outside current library)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants