-
Notifications
You must be signed in to change notification settings - Fork 213
Constant constructor call should be legal in another constant constructor definition. #823
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
Comments
Treating "const" as "new" in some cases would probably be too confusing. It is a problem that you can't create an object in a const constructor initializer list. In the long run, a solution could be to make "new" optional, so you write: Added Area-Language, Triaged labels. |
This comment was originally written by @Cat-sushi Yes, treating "const" as "new" is confusing. The villain of the confusion is newable constant constructor. Of course, newable const constructor is useful and kind of reasonable. |
This comment was originally written by @Cat-sushi In addition, on the other hand, my proposal is completely consistent in compile-time constant context, like other initialization list. I think, it is kind of reasonable. // 'call C(x);' in my original post is typo of 'const C(x);'. (^_^) |
This comment was originally written by @Cat-sushi I think the current implementation compiles constant constructor without call site sequence. I think it is the bottle neck of solution of ambiguity in potentially constant expression. |
Added Customer-Flutter label. I'd like to be able to define Example: class Semantics extends StatelessWidget {
// This is not supported today:
const Semantics({
bool checked: false,
bool selected: false,
}) : properties = const SemanticsProperties(
checked: checked,
selected: selected,
);
const Semantics.fromProperties(this.properties);
final SemanticsProperties properties;
} |
FWIW, this is forcing Flutter to have a breaking API change. |
Summary: - Add `key` field to `SemanticsNode`, while moving key into `foundation` library so it can be used by the render layer. - Introduce `SemanticsProperties` and move many of the `Semantics` fields into it. - Introduce `CustomPaintSemantics` - a `SemanticsNode` prototype created by `CustomPainter`. - Introduce `semanticsBuilder` and `shouldRebuildSemantics` in `CustomerPainter` **Breaking change** The default `Semantics` constructor becomes non-const (due to https://github.com/dart-lang/sdk/issues/20962). However, a new `const Semantics.fromProperties` is added that still allowed creating constant `Semantics` widgets ([mailing list announcement](https://groups.google.com/forum/#!topic/flutter-dev/KQXBl2_1sws)). Fixes #11791 Fixes #1666
Summary: - Add `key` field to `SemanticsNode`, while moving key into `foundation` library so it can be used by the render layer. - Introduce `SemanticsProperties` and move many of the `Semantics` fields into it. - Introduce `CustomPaintSemantics` - a `SemanticsNode` prototype created by `CustomPainter`. - Introduce `semanticsBuilder` and `shouldRebuildSemantics` in `CustomerPainter` **Breaking change** The default `Semantics` constructor becomes non-const (due to https://github.com/dart-lang/sdk/issues/20962). However, a new `const Semantics.fromProperties` is added that still allowed creating constant `Semantics` widgets ([mailing list announcement](https://groups.google.com/forum/#!topic/flutter-dev/KQXBl2_1sws)). Fixes flutter#11791 Fixes flutter#1666
I think it can be closed with dart 2.0. |
@Cat-sushi I'm still seeing the following analyzer error: "Initializer expressions in constant constructors must be constants." |
@yjbanov So am I. |
I also see the error in DartPad and with Constant expressions in Dart were designed to be significantly less expressive than regular Dart. In particular, there's no notion of value directed recursion (which would allow for constant expression evaluation with high complexity, e.g., like the Ackermann function). A simple example would be the following: class A {
final A next;
const A(int i): next = i > 0 ? const A(i - 1) : null;
} As mentioned in earlier comments on this issue, we might be able to omit But if we were to support this kind of recursion then there's basically no limit on the amount of computation that any given constant expression could give rise to, and that might well cause a perceived startup time optimization to turn into a program size nightmare (if we'd compute some huge object graphs at compile-time and make them part of the shipped program). So the rules were kept simple: For a 'constant expression' there is a tight relationship between the syntax and the semantics (one syntactic expression corresponds to one constant object at run-time, and, by canonicalization, one constant object corresponds to all the syntactic expressions that have "the same value"). For a 'potentially constant expression' Dart only allows certain built-in operations (like Even C++, whose template feature is known to be Turing complete, has a notion of constant expression evaluation ( In summary, there are reasons why a constructor like This issue should not be closed based on the updates in Dart 2 (Dart 2 doesn't differ from Dart 1 in any way that makes a difference for this issue). It could be maintained as a request for some enhancement in the area of constant expression evaluation, or it could be closed because such enhancements are not trivial to introduce (nor is it guaranteed that they have widespread support). I basically think that the situation is unchanged, and if you, @Cat-sushi, or anyone, wish to maintain the pressure to get some enhancements in this area then this issue is a fine place to have that discussion. |
@eernstg thanks for explaining the background. I understand that there're a lot of cases where this could get nasty if implemented carelessly. However, I also think that there're a lot of valuable features that would be in the safe zone if we think carefully about it. |
I just checked whether I could find a "canonical" issue on enhancements to constant expression features, but it seems like there is no such thing. We have other requests like dart-lang/sdk#29277 about functions that could be invoked during constant expression evaluation (so that's related, but much more radical). I think it will be difficult to extend the expressive power of constant expressions without letting it explode into something which is essentially the whole language, but it could still be worthwhile to look for simple restrictions that would be strong enough to avoid that, and still useful in common, concrete scenarios. |
There's also my ancient DEP: https://github.com/yjbanov/dart-const-functions |
Right, I was actually looking for that, but searching issues, and didn't find it. Thanks! ;-) |
I'm not sure how relevant those motivating examples are any more (@matanlurey can bring us up to date), but there's plenty over in the Flutter land. |
I don't see any reason why this would be 'customer-flutter' only, but compelling examples from Flutter land would certainly be relevant. |
I don't believe the labels mean that a particular customer is the only one who's interested in a feature, only that the customer really really cares about the issue :) |
@eernstg Stumbled across this issue from "Flutter land" & am hoping highlight a use-case. Example class CustomWidget {
// ...
CustomWidget({
this.borderRadius = CircularBorderRadius.zero, // <—— "error: Default values of an optional parameter must be constant."
});
// ...
} class Radius {
final double radius;
const Radius(this.radius);
}
class BorderRadius {
final Radius x;
final Radius y;
const BorderRadius(this.x, this.y);
}
class CircularBorderRadius extends BorderRadius {
static const CircularBorderRadius zero = const CircularBorderRadius(0.0);
const CircularBorderRadius(double radius):
super(const Radius(radius), const Radius(radius)); // <— ERROR "Arguments of a constant creation must be constant expressions."
} |
@mwalkerwells: Thanks for the input! That is indeed an interesting case, because it may seem reasonable to be able to create two constant But it's a tricky step to take! ;-) The example fails now because of the nature of Dart constant constructors: There is nothing stopping them from being invoked at run time (passing arguments that are not known at compile-time), so there is no way you could ever be allowed to have So in order to get started on such a feature we would need to introduce the notion that "this particular That shouldn't be too hard to support, but it might not be considered to carry its own weight, weighed up against the notational and possibly semantic complexity of allowing developers to specify such a thing. Even then, the ability to write things like So we'd be back in a situation where we couldn't promise that constant objects are manageable for large projects, because they might suddenly explode in size (and it might be difficult to see why). So even though that looks like a small step yielding an obviously useful feature, it's actually a feature that opens the door to all the complex stuff that we decided long ago we wouldn't allow in the context of constant computations. So, if we are to go down this path, we'd at least need to confine such a feature very carefully in order to keep the generalization less powerful than general recursive functions. |
@eernstg Thank you so much for such a thoughtful explanation! |
I know this is an old issue, but I'd like to raise a point. I understand @eernstg where you are coming from when you point out that users can use recursive functions in their Unless if my understanding of |
Also, as to how Dart would determine if the call is a class A { final int value; const A(this.value); }
|
@Levi-Lesches wrote:
Right, run-time resource consumption generally cannot be bounded in a Turing complete language. But Dart puts a bound on constant expressions for several reasons, including the fact that this makes constants "safe" even in large programs, in the sense that the size of the set of constant objects won't explode.
I think the emphasis put on using constants by people who are keeping an eye on their performance indicate that it is worthwhile.
Yes, but that's exactly because 'constant expression' is defined inductively (using rules that generally have the form "e1+e2 is constant if e1 is constant and e2 is constant [plus any additional requirements]"), which means that it is a quite simple procedure to tell whether a given expression is constant. And |
Thanks for responding. As for your first point, say the memory for the constant objects do explode. What benefit does the compiler get by forcing that memory bloat to run-time instead of compile-time (and recreating the objects each time)? I get that a long compile-time is a good warning that something is wrong, but most programmers know that so is a long delay on a button press. As for determining what is and isn't a constant, my point still stands that this wouldn't introduce complications since Dart easily knows what constants are. By allowing more constructors to be |
I agree that accepting a long compile step in return for better performance at run time is a perfectly meaningful choice—basically enabling non-trivial optimizations on any compiler is an example of doing that. But this is not just about having a large program that suddenly takes a long time to compile. It's also about having many pieces of software which are brought together in order to create a large system. If constant evaluation complexity isn't bounded (e.g., constant computation could be Turing complete just like C++ template metaprogramming or Space consumption at run time is of course equally bad in several ways, but not all: With a big constant pool you'll have a bigger download size; also, with dynamically created objects you may create a big heap at some point, but garbage collection may be able to delete most of that as soon as the memory has been exhausted, and even if that is not possible it would not have helped to make them constant. Besides, I mentioned that we've kept constants relatively simple for several reasons. One of them is that it's not simple at all ;-) For instance, there are about 2400 github issues referring to In any case, requests for more general constant expressions keep coming up, so we are certainly aware of the fact that it is an area of interest. |
Now that we have NNBD, is this issue, along with #663, at sight? |
Just to highlight another use-case, this doesn't work right now: class MyIcon {
const MyIcon._(int codePoint) :
outlined = IconData(
codePoint,
fontFamily: 'MyIconsOutlinedRounded',
fontPackage: 'this_package',
),
filled = IconData(
codePoint,
fontFamily: 'MyIconsFilledRounded',
fontPackage: 'this_package',
);
final IconData outlined;
final IconData filled;
}
@immutable
class MyIcons {
const MyIcons._();
static const add = MyIcon._(0xf101);
static const addressBook = MyIcon._(0xf101);
// hundreds more of these
} Instead, we're forced to do this: class MyIcon {
const MyIcon._(this.outlined, this.filled);
final IconData outlined;
final IconData filled;
}
@immutable
class MyIcons {
const MyIcons._();
static const add = MyIcon._(
IconData(
0xf101,
fontFamily: 'MyIconsOutlinedRounded',
fontPackage: 'this_package',
),
IconData(
0xf101,
fontFamily: 'MyIconsFilledRounded',
fontPackage: 'this_package',
),
);
// ...
} This increases the file size from a mere 1000 to over 8000 lines of code. Also, constant evaluation, in this case, is a necessity so that tree-shaking of the icon font works. |
See #2256 where this issue was first raised: A constructor in an enum declaration (as of Dart 2.17, where enhanced enum declarations were introduced) is guaranteed to be invoked as part of constant expression evaluation. In other words, they will never be evaluated with This means that some of the concerns stated above (involving instance creation at run time) do not apply to that kind of constructor, and hence it might be easier to support something like the proposal of this issue in that special case. |
I think it's appropriate to include construction of objects in a slightly broader sense than instance creation expressions (that is, expressions invoking a constructor, like In particular, we could cover collection literals as well in this issue. Here is a motivating example: class MyApi {}
class ClassProvider {
final Type type;
const ClassProvider(this.type);
}
class FactoryProvider {
final String token;
final String Function() func;
const FactoryProvider.forToken(this.token, this.func);
}
class Module {
final List<Object> providers;
const Module(this.providers);
}
class MyApiModule extends Module {
const MyApiModule(String Function() keyFunction) : super(const [
ClassProvider(MyApi),
FactoryProvider.forToken('key', keyFunction), // *** Error: `keyFunction` is not constant. ***
]);
}
void main() {
const module = MyApiModule(myFunc);
print(module.providers);
}
String myFunc() {
return 'test';
} In short, the fact that class MyApi {}
class ClassProvider {
final Type type;
const ClassProvider(this.type);
}
class FactoryProvider {
final String token;
final String Function() func;
const FactoryProvider.forToken(this.token, this.func);
}
class Module {
final List<Object> providers;
const Module(this.providers);
}
class MyApiModule extends Module {
const MyApiModule(super.providers);
}
void main() {
// We have to provide the entire constant expression here:
const module = MyApiModule(const [
ClassProvider(MyApi),
FactoryProvider.forToken('key', myFunc),
]);
print(module.providers);
}
String myFunc() {
return 'test';
} |
Allowing potentially constant/non-singleton object creation in a It breaks with one rule that Dart has so far maintained: That a If we allow such a potentially constant object creation, then we can't rely on our current, rather simple, cycle detection, where we check whether the evaluation of a const expression depends on its own value. If the same expression can have multiple values, we either have to say that a constructor cannot depend on itself, or we have to accept non-linear object creation. Say: class CFib {
final List<CFib> value;
const CFib(int n) : value = n <= 2 ? [] : [CFib(n - 1), CFib(n - 2)];
}
var x = const CFib(48); This constant evaluation terminates (eventually) and creates ... ~48 objects (after canonicalization, ~4G before). (Could be worse. Could be the busy beaver function.) I'm sure I can find a way to actually create the 4G different objects too. if necessary. Just do some string concatenation along the way. |
Thanks for including my example! I wanted to include some additional context around the use case and why pulling the values out to be passed in by the caller doesn't work. The goal here is to allow for a private token in AngularDart's dependency injection (DI) framework (I cheated in my example by making the "token" a String because I didn't want to define another new class, so imagine it's an OpaqueToken), which would prevent consumers from accessing the private implementation of that token's DI value. Since AngularDart's DI framework relies on const modules, the constructors must be const. However, that also prevents defining a module that takes a constructor parameter and uses it to build its provider list. As a result, the way this would have to work is
After more thought, if you're using untrusted third-party packages then perhaps this becomes important, because you want to make sure those third-party packages can't inject the values meant for another package that may contain secrets (like developer keys). |
@creisman, here's another variant of the example: class MyApi {}
class ClassProvider {
final Type type;
const ClassProvider(this.type);
}
class FactoryProvider {
final String token;
final String Function() func;
const FactoryProvider.forToken(this.token, this.func);
}
class Module {
final List<Object> providers;
const Module(this.providers);
}
class MyApiModule implements Module {
final String Function() _keyFunction;
List<Object> get providers => [
const ClassProvider(MyApi),
FactoryProvider.forToken('key', _keyFunction),
];
const MyApiModule(this._keyFunction);
}
void main() {
const module = MyApiModule(myFunc);
print(module.providers);
}
String myFunc() {
return 'test';
} In this variant we're pushing the construction step in the opposite direction, computing it when This may not be acceptable: We don't have object identity among the lists returned by this We could of course use something like a top-level map to cache the returned list (such that we can reestablish the "same object every time" property, and perhaps save some time, and definitely save some space), but that's also an extra chore that we might not want to deal with as the application scales up. However, if it is acceptable to do one of these things, then it is a possibility to construct the composite objects late. |
Unfortunately the reason these modules have to be const is because AngularDart DI is resolved at compile time using code generation. While your implementation would have worked in the older, reflective implementation of DI, my understanding is it's not compatible with the new compile-time implementation. |
For what it's worth, code generation is already heavily used in Dart and macros will allow Dart code to generate unbounded constant objects at compile time. Given that, I'm not sure if it's super important to maintain this restriction for const. |
@creisman wrote:
Actually, For instance, If the problem is that class MyApi {}
class ClassProvider {
final Type type;
const ClassProvider(this.type);
}
class FactoryProvider {
final String token;
final String Function() func;
const FactoryProvider.forToken(this.token, this.func);
}
class Module {
final List<Object> providers;
const Module(this.providers);
}
class MyApiModule extends Module {
final String Function() _keyFunction;
List<Object> get providers => [
const ClassProvider(MyApi),
FactoryProvider.forToken('key', _keyFunction),
];
const MyApiModule(this._keyFunction) : super(const []);
}
void main() {
const module = MyApiModule(myFunc);
print(module.providers);
}
String myFunc() {
return 'test';
} This does waste a little bit of space (because an instance of But perhaps the entire class |
It probably would have helped if I'd discussed the code generation implementation a little more (I'm not an expert in this area, so take what I say with a grain of salt). I think it was probably inaccurate for me to describe it as being compile-time, since I think the code generation technically happens prior to the compilation step. It works by using the AST to identify the @GenerateInjector annotation, which takes a list of providers/modules as an argument. The AST processor is able to parse the list of providers passed to the module and use it to generate a new source file that contains the DI implementation that is actually consumed at runtime. So the module structure that's written by developers is, to my knowledge, completely ignored at runtime. This is another case of my simplified example being misleading in its desired output, sorry about that :(. I was printing the provider list in the example only to test the resolution of the const evaluation (which failed, so it didn't even matter). Here's the AngularDart DI code generator that runs on these modules. Here's the module parser. Note that the syntax for Modules is slightly different from my example. They have two named parameters (include and provide) rather than a single positional parameter. |
There's still an error, unless I'm missing something. e.g.
|
This issue was originally filed by @Cat-sushi
The code below is illegal with the current language specification.
class C {
final x;
const C(this.x);
}
class D {
final C c;
const D(x) : this.c = const C(x); // compile-time error!
}
I understand that 'x' in 'const C(x);' is not a compile-time constant, but a potentially constant expression, and this is the reason why the constructor call 'call C(x);' is illegal.
I also understand the reason why the 'x' is potentially constant, the constantivity of 'x' is depends on the call site which calls the constructor whether with 'const' or with 'new'.
On the other hand, with my understanding, the compiler evaluates the result of constant expressions at compile-time, and the compiler can check constantivity at compile-time with call site sequence.
At the same time, in run-time context, constructors are less constrained, and the language processor can simply regard 'const' of 'const C(x);' as 'new', and can call the prepared run-time constructor.
I think the disregard of keyword 'const' and the need for the run-time constructor of the constructor call 'const C(x);' in run-time context are kind of similar to those of constant constructor definition like 'const C(this.x);' or const 'D(x)...;'.
So, regardless of discussion in issue dart-lang/sdk#392 and issue dart-lang/sdk#19558, I think constant constructor calls in another constant constructor definition should be legal.
And, I feel it is more natural.
In addition, I think this proposal is upper compatible of the current language specification.
The text was updated successfully, but these errors were encountered: