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

Annotation that the argument for a parameter should be a constant #29381

Closed
rakudrama opened this issue Apr 18, 2017 · 11 comments
Closed

Annotation that the argument for a parameter should be a constant #29381

rakudrama opened this issue Apr 18, 2017 · 11 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). core-n type-enhancement A request for a change that isn't a bug

Comments

@rakudrama
Copy link
Member

Intl.message has an examples: parameter. See https://www.dartdocs.org/documentation/intl/0.14.0/intl/Intl/message.html

The Map parameter is unused at run time.
The documentation does state: "The examples map must be a valid const literal map".
However, users don't always remember the const. It is very expensive to create a map for each call to message().

What we need is a way to enforce that the map is a constant, perhaps an annotation on the parameter that the corresponding argument at the call site must be a constant.

@bwilkerson bwilkerson added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Apr 18, 2017
@bwilkerson
Copy link
Member

If we don't want to add language support to specify that a parameter is required to be a constant value (such as by allowing the const modifier to be applied to parameters), then, yes, we could add an annotation for it.

@matanlurey
Copy link
Contributor

Is there anyway to repurpose @literal for this? @bwilkerson

It might be confusing to have multiple annotations that all mean "please use const".

@bwilkerson
Copy link
Member

I wouldn't have a problem using @literal on parameters, though I presume that when used for a parameter it wouldn't have the same caveat.

@lrhn lrhn removed the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Jun 25, 2018
@lrhn
Copy link
Member

lrhn commented Jun 25, 2018

Constant objects in Dart are not intrinsically special. They are objects just like any other instance of the same class. They may have been created at runtime and then canonicalized (that's what the dev-compiler does).

It's easier to say that an expression must be a constant expression than it is to say that a value must be constant. It's technically possible to mark a parameter as const and then require that all arguments to it are constant. However, it complicates function types. A void Function(const List) must be a super-type of void Function(List) (A function accepting List can be used wherever a function expecting const List is expected, beacuse you can pass constant lists to both).

Generality would also suggest that const should be available for all type annotations, like a function that can only return constant objects, or a variable that can only hold constant objects (with List being a super-type of const List).

All in all, I don't see the benefit of such a feature as outweighing the costs in complexity.

@lrhn lrhn added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug labels Jun 25, 2018
@lrhn lrhn changed the title pkg/meta: annotation that the argument for a parameter should be a constant Annotation that the argument for a parameter should be a constant Jun 25, 2018
@natebosch
Copy link
Member

Have we decided that this isn't worth a change in the language? Should we move back to area-analyzer to implement a static check outside of the language?

@lrhn
Copy link
Member

lrhn commented Sep 3, 2018

We have not decided either way, but it hasn't really been high enough on our list of priorities to be discussed at all.

I can see the reasoning for wanting to give the user a hint that something should probably be constant, but it doesn't really scale to a language feature.

If we mark a method or function parameter declaration as requiring a constant argument, then what happens if we want to treat that function/method-tear-off as a first class function value? Should it retain the property that you can only call it with a constant argument?

If not, it doesn't feel like a proper language feature, more like a hint or suggestion.

If so, then we need "constant only" parameters in function types, and some subtyping relation between constant-requiring functions and non-constant requiring functions.
Likely the constant-only function would be a super-type of the more general function type (in the traditional contravariant manner where a function allowing more arguments can be used as one allowing fewer).

That means you can't call zone.runUnary((const List<int> l) => print(l), const [1]) because the type inference would infer it as zone.runUnary<void, List<int>>((const List<int> l) => print(l), const [1]) and the function literal isn't actually assignable to void Function(List<int>), which would be the argument type. That seems fairly problematic.

Would we then need to allow const List<int> as a type argument? Then it's not just function types that are affected any more, we get a complete type hierarchy of const values parallel with the non-const types.

It would probably work, but the complication of the type system is more costly than I think the feature is actually worth, so I don't see this actually happening. This would likely be more intrusive than const in C++, without actually giving us the "can be evaluated at compile-time" guarantee for a function that must return a constant value.

So, after this contemplation, I don't see this as a viable language feature for Dart.

Adding it as an analyzer hint does seem more reasonable. It's not required to be safe or complete, just to heuristically help people do the right thing in the common case.

@lrhn lrhn added the core-n label Dec 6, 2018
@bwilkerson
Copy link
Member

Do we want to be able to apply the annotation to an operator (int operator+(@mustBeConst p) ...)? It would be consistent to allow it, but I'm not sure there's much value.

Do we want to be able to apply the annotation to a setter (void set(@mustBeConst p) ...)? It could be an interesting way to ensure that a field's value is always a constant, but again I'm not sure there's much value.

@mosuem
Copy link
Member

mosuem commented Mar 15, 2024

The CL for this feature is here in case of any comments.

@lrhn
Copy link
Member

lrhn commented Mar 15, 2024

I would apply the annotation to operator and setter parameters.
Operators are just methods with funny names, and special calling conventions. And just omitting setters feels inconsistent.

Consider whether we'd want:

@mustBeConst
String id = "";

to apply the @mustBeConst to the implicit getter. (That's what we do with other parameter modifiers, like covariant.)

@dcharkes
Copy link
Contributor

@mosuem and I discussed whether we want propagate consts, and this can lead to a space explosion as detailed in (dart-lang/language#2776 (comment)).

Also, when doing propagation, we quickly get into the question whether control flow should be supported and whether string concatination or any other const-able operation should be supported as discussed above. These can lead to more space explosion.

For now, the use cases in dart-lang/native#153 don't seem to need it.

copybara-service bot pushed a commit that referenced this issue Apr 4, 2024
constant.

As const-only parameters are not planned, see
dart-lang/language#1684, an annotation with analyzer support could help a bit at least, see
#29381.

The motivation is to enforce const arguments for methods annotated with
`@ResourceIdentifier`, to be able to record the argument values at build time, see https://dart-review.googlesource.com/c/sdk/+/329961.

Tested: pkg/analyzer/test/src/diagnostics/const_argument_test.dart
Change-Id: I2b8d2dce0c899fc0caa4985d892a5d031c747521
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357701
Reviewed-by: Phil Quitslund <[email protected]>
Reviewed-by: Lasse Nielsen <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Moritz Sümmermann <[email protected]>
Reviewed-by: Marya Belanger <[email protected]>
@matanlurey
Copy link
Contributor

This was landed in b321254.

In the next Dart release (or sooner if you use an unstable release), the analyzer should trigger for @mustBeConst.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). core-n type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants