dart-lang / language

Design of the Dart language
Other
2.65k stars 202 forks source link

Eliminate unnecessary constraint on metadata? #639

Open eernstg opened 4 years ago

eernstg commented 4 years ago

Cf. dart-lang/sdk#39077.

Metadata is subject to a special constraint. With @e, the following rule applies:

It is a compile-time error if $e$ is not one of the following:
\begin{itemize}
\item A reference to a constant variable.
\item A call to a constant constructor.
\end{itemize}

In particular, e cannot be a reference to a top-level function or a static method, even though that does make e a constant expression.

However, it is not obvious to me that this constraint is helpful (to anybody). We might as well simplify the specification by deleting the above 5 lines, such that e in @e can be any constant expression, as long as @e is derivable from <metadata>.

The analyzer currently reports an error on the following, but the common front end (and, in particular, dart and dart2js) compile and run it without issues:

@V.tearOff
class V {
  static tearOff() {}
}

topLevelTearOff() => 4;

@topLevelTearOff
class W {}

main() {}

It should not be hard to implement support for arbitrary constant expressions that satisfy the grammar rules for metadata—after all, we can certainly have the following (where the torn off function is used in metadata, just nested):

topLevelTearOff() => 4;

class A { 
  final Function f;
  const A(this.f);
}

@A(topLevelTearOff)
main() {}

The current situation is that the tools disagree. We could change the common front end to report the error (as requested in dart-lang/sdk#39077), or we could change the analyzer to allow constant expressions in general as proposed above.

This issue serves to describe the situation, and to suggest that we do the latter (because it makes Dart simpler and more consistent, and the constraint does not seem to improve on anything).

This change would be low-priority, it's just a matter of deciding whether to add the error to the common front end or removing it from the analyzer. If it does not make much difference we might as well remove it from the analyzer. If so, we shouldn't rush ahead with dart-lang/sdk#39077.

lrhn commented 4 years ago

One reason to retain the restriction is to avoid someone accidentally forgetting the parentheses after a constructor. Say:

@pragma
int foo() => 42;

This is currently invalid because pragma is a class name, not a variable. If it was valid, you could do this an expect it to be a pragma annotation, where it is in fact a Type object annotation.

Then, others will argue that it's exactly what they want for annotations with optional parameters, so you can do both:

  @deprecated

and

  @depreated("for a reason!")
ds84182 commented 4 years ago

Having this for dart:ffi would make declaring structs a bit cleaner:

class Foo extends Struct {
  @Int32 int bar; // Instead of @Int32(), where the parenthesis add nothing but noise to the already noisy signature.
}

Int32 is a class because of NativeFunction signatures: Int32 Function(Int32) and it defines a const constructor so it can be used as an annotation also. It may be fairly confusing to someone because the declaration of Int32 also feels like it could be a value type for a 32-bit int, especially because these native types are used as part of virtual function signatures.