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

Loosen const requirements for asserts #4260

Open
filiph opened this issue Feb 11, 2025 · 3 comments
Open

Loosen const requirements for asserts #4260

filiph opened this issue Feb 11, 2025 · 3 comments
Labels
request Requests to resolve a particular developer problem

Comments

@filiph
Copy link

filiph commented Feb 11, 2025

Currently, everything in a const constructor must be a compile-time constant, including the asserts:

class Data {
    final String value;
    const Data(this.value)
        : assert(value == "banana"); // okay
}

This limits the usefulness of assert(). Often, I get into a situation where I'd love to have a constant class but also still want to check that the constructor arguments are valid when the class is constructed at runtime. Like so:

class Data {
    final String value;
    const Data(this.value)
        : assert(value.contains("banana"); // ERROR: Invalid constant value. invalid_constant
}

The above example comes from someone's StackOverflow question. Here's another example that is closer to what I needed just a minute ago:

class MyWidget extends StatelessWidget {
  final MyBox box;
  const MyWidget(this.box)
      : assert(box.nonFinalItem != null);  // ERROR: Invalid constant value. invalid_constant
}

Today, my only solution is to either move the assert to somewhere else, or make the class non-const.

  1. Moving the assert somewhere else is often not Fail-fast enough (e.g. the assert is raised way later or never). On the other hand, if it was raised during the constructor call, I'd immediately see who's trying to pass the invalid argument from the stack trace, which is great for debugging.
  2. Making the class non-const just because we want to assert in a constructor seems silly. Constant classes have performance and semantic advantages over non-const classes.

I understand that an assert like box.nonFinalItem != null can't be run at compile time, and therefore can't be const.

But as a developer, when I'm writing an assert like that, I'm not expecting the assert to be checked at compile time. I expect it to be checked at runtime, when the widget is created dynamiclly.

Edit: Now that I write it down, I'm realizing maybe the fact that we have any non-final fields in any of the parameters means that the class cannot ever be instantiated at compile time as a constant? And therefore marking such class as const is silly in itself?

I wonder if it's possible to loosen the requirement of const-ness in asserts. Can non-const asserts be permitted to be attached to const constructors, with the understanding that they won't be run at compile time?

@filiph filiph added the request Requests to resolve a particular developer problem label Feb 11, 2025
@lrhn
Copy link
Member

lrhn commented Feb 11, 2025

This would add a second kind of asserts to const constructors, which are not run at all when const constructed, but will run when non-const constructed.

Let's strawman it as 'runtimeAssert' '(' <expression> [',' <expression>] ')'.
It can only exist in non-redirecting generative const constructor initializer lists, because that's the only place
where an assert can be const-evaluated at all.

Currently an assert is executed if assertions are enabled or it is being run as const (which only happens inside a const constructor),
and a runtimeAssert would then be executed if assertions are enabled and it is not being run as const.

It's an assertion, it shouldn't change the behavior of the constructor. It can, but it shouldn't have side effects.
If it does, those side-effects will only happen when run as non-const.

That's still the biggest issue with this function. Currently const constructor invocations can run the same constructor code as non-const invocations (I don't know if the VM still does this), just with asserts being enabled.
Because the code inside the assert must be potentially constant, it cannot do anything that would invalidate
assumptions about potentially constant code.

With a runtime assert, you can write:

  final (int, int) pair;
  const Foo(int x, int y) : runtimeAssert(++x == x && --y == y), this.pair = (x, y);

A const constructor can ususally assume that its parametres cannot change because assignment is not a valid potentially constant expression. It can effectively assume that all parameters are final. If we can run non-potentially-constant expressions during execution, then the compiler can no longer assume that, and it may need to change its implementation. Maybe that's not a problem, but it does need to be checked and tested.

And you can create closures over the scope:

  const Foo.capture(int x, int y) 
      : runtimeAssert(_escape = ((int dx, int dy) => (x += dx, y += dy)) != null),
        runtimeAssert(_escape?.call(10, 20)) != null),
        this.pair = (x, y);
  static (int, int) Function(int, int)? _escape;

The compiler can now see x and y being assigned to inside a closure, and it won't know when that happens.
It can happen at any time after that closure is created. Again the compiler can no longer assume that constant
constructors are side-effect free. And again, it's probably not a problem, it's still safe during constant evaluation,
since those non-constant expressions are never evaluated, but it might break some assumptions that compilers are safely making today.

And if you want to be very clever, it can be used to tell whether an invocation is const or not,
and do different things, like creating a new runtime object if possible, and use a default if const.

  const factory Discern(int x, int y) = Discern._;
  const Discern._(int x, int y, [bool isConst = true, String? _betterString])
    : runtimeAssert(
          (isConst = false) || (_betterString = "({$x.toRadixString(16)}, ${y.toRadixString(16)})" != null)),
      this.text = _betterString ?? "($x, $y)",
      this._isConst = isConst;

  final bool _isConst;
  String toString() => "${_isConst ? "const ": "")Discern($x, $y)";

I would totally use this!
I too have classes where I want to do validation of input, but also want to be able to be const,
and then I'll have to do validation at runtime instead when the object is first used
(because I can't safely do no validation). Take

const factory RegExp(this.pattern, {...flags...}) = RegExp._;
const RegExp_(this.pattern, {...flags..., bool isConst = false, _CompiledRegExp? _compiled}) 
    : runtimeAssert(_compiled = _validateAndCompilePattern(pattern, ...flags..) != null),
      this._compiled = _compiled,
      // ...

final _CompiledRegExp? _compiled;
static _constCompilationCache = Expando<_CompiledRegExp>();
_CompiledRegExp get _ensureCompiled => _compiled ?? 
     (_constCompilationCache[this] ??= _validateAndCompilePattern(pattern, ...flags..);

Which probably means it's not a good idea, or not a well-focused idea, if I'm going to exploit it for something
completely different from what it was intended for.

It's a powerful feature in several ways, which probably means that thinking of it as only for asserts is underselling it. This is about distinguishing a const invocation from a non-const invocation, and allowing the non-const
invocation to run non-const code. Maybe the feature we want should do that directly:

So, consider instead this feature (strawman-syntax!): A const generative constructor invocation can evaluate the keyword const as an expression which evaluates to true if the constructor invocation was const and to false if not.
One might need to write it as (const) inside other expressions, maybe require it to always be parenthesized. Syntax is hard.
If a condition is the const value, then the context is definitely constant on the true branch and definitely not-constant on the false branch. In a context that is definitely not constant, any expression is allowed.
If the context is definitely constant or potentially constant, only potentially constant expressions are allowed.
(A test like (const || value == nul) ? ... : ... would have a potentially constant context for the then branch, definitely non-constant for the else branch. I don't think there is ever any distinction beween definitely and potentially constant. Either we know it's not constant, or we don't, and then have to assume that it is.)

It's like a promotion of the context to "not constant" which only happens if all paths leading to the code has ensured that const is false, and should be possible to be integrated into normal flow analysis.

With that, you can do runtime asserts as:

assert(const || (any.expression.here));

which is satisfied trivially during const evaluation, and does more during a non-const invocation.

I can get my _compiled = const ? null : _validateAndCompilePattern(pattern, ...flags...) without the extra hacks,
and without needing to assign to parameters and potentially confuse a compiler into making worse code.

Can maybe even use that const also forces evaluation of asserts, and allow asserts to promote on const
branches.

  final int x;
  const Foo([int? x]) : assert(x is int), x = const ? x : (x ?? 0);

Here the compiler knows that assert is definitely run if const is true, so the const expression is a boolean
expression which implies the promotions performed by every assert expression before it, at least their const
branches.
Something like int y; Foo(int? x) : assert(x is int), y = const ? x : (x ?? _computedDefault());
can act, wrt. promotion like it was similar to:

void foo( final bool isConst, bool assertionsEnabled, [int? x]) {
  int y;
  bool b = (isConst || assertionsEnabled) && (x is int || (throw AssertionError("nope")));
  y = (isConst && b) ? x : (x ?? 0);
  // ...
}

which does promote x to int in the true branch.
(Any const expression is implicitly equivalent to const && c1 && ... & cn where c1..cn are the conditions of
all prior asserts, because we know all prior assert conditions have been evaluated to true if const is true.)

Such a (const) flag to const constructors is definitely powerful and useful.
Not sure this is something we'd want to add. People already have a hard time remembering what can be constant and what cannot, and the separation into constant constructor and non-constant constructors, with all the requirements being on the constant constructor, is a (fairly) straight line in the sand. Introducing a "conditionally constant" constructor, which is more of a way to have two constructors with the same name, overloaded on the const-ness of the invocation, might just make things more complicated.

So another option is to have two versions of the constructor, with the same name, and overloading on const.

  const Foo(int x, int y) : assert(...), this.pair = (x, y);
  new Foo(int x, int y) : assert(non-const-checks), this.pair = _computePair(x, y);

It's overloading, but only on const/new.
We can require that the two constructors have the same parameter list. (Or not.)
Or we can combine them even more:

  const Foo(int x, int y) 
      : assert(...),
        this.pair = (x, y),
      new:
        assert(non-const-checks),
        this.pair = _computePair(x, y);

There are two completely separate initializer lists, one for const and one for new.
If you only write one, it must be the const one for a const constructor, but you can
have a new initializer-list too, which is used when invoked without new.
(That's also the functionality that a tear-off will run.)
Compilers can optimize this under the hood to execute the const initializer list at
compile-time, and only compile the new list into the runtime program.

Less strictly powerful (might cause some duplication, cannot be used in a redirecting
generative constructor, which (const) could, unless those also have to branches:

  const Foo.redir(int x, int y) : this(x, y), new: this._runtime(x, y);

so that it redirects based on const-ness. (Can do that for factory constructor too:

const factory Foo.facredir(int x, int y) = Foo, new: Foo._runtime;

Syntax is not awesome. It's also not awesome to have a slightly complicated language feature for conditional redirection, and then only use it for const. What if it could redirct based on any condition, and then (const)
was just one condition you could use:

 const Foo.redirc(int x, int y): if (const) this(x, y) else this._runtime(x, y);

(Using if because it's a "conditional redirection", not a conditional expression.)

Without any new language feature, current solutions are indeed limited.

You can have only a non-constant constructor. Which makes it impossible to create default values or other values that need to be const, hence this request.

You can have two constructors:

  const Data.unchecked(this.value);
  Data(this.value) : assert(value.contains("banana"));

But then, why use an assert at all, if you can get the validation at all times, and throw a better error?

  Data(this.value) {
    if (!value.contains("banana")) {
      throw ArgumentError.value(value, "value", "Has no banana!");
    }
  }

(You can get the better error too with a non-constant assert, as

assert(value.contains("banana"), throw ArgumentError.value(value, "value", "Has no banana!"))

but you can't with a constant assert. I'd like to change that, make it so that the second expression
of an assert does not have to be a constant expression. If it is reached during constant evaluation,
it should just be an immediate compile-time error, and at runtime it can choose what to do.)

Nothing prevents using new Data.unchecked("pineapple"), but then, nothing prevents const Data("pineapple") either, if the validation is only a runtimeAssert. (Or just new Data("pineapple") with asserts disabled. I don't recommend having important validation only as asserts. I have a hack for that, adding throws to potentially constant expressions that run if even if asserts are disabled, but it's an ugly hack.)

You can just not validate. Which has its own issues if the validation is important. (But then it shouldn't be only an assert, that just means you can get an invalid state in production from dynamically provided values.)

Another alternative would be extending the abilities of constant evaluation, fx to include some of the following possible operations (always with potentially constant receiver and arguments that evaluate to instances of platform types - mainly for lists/sets/maps):

  • String.contains, String.substring, String.startsWith, String.endsWith.
  • int.isEven, int.isOdd, int.remainder (we allow %, but not the remainder that's compatible with ~/!),
  • double.{floor,ceil,truncate}, double.isNaN, double.isFinite,
  • num.sign, num.compareTo.
  • List.contains, Set.contains, Map.containsKey, Map.containsValue if receiver is platform list/set/map and elements/keys/values have constant/primitive equality.
  • List/Set/Map's length, isEmpty, isNotEmpty.
  • List.[], Map.[], List.first, List.last.
  • ... and more.

That won't ever solve the problem, it'll just push the limits out a little further. The next request will be to support something like list.every((x) => x.isEven). That's why we're somewhat reluctant to start adding arbitrary functions.
We should either add all reasonably constant operations of int/double/String/bool/List/Set/Map,
those which can be computed for constant platform values without running user code, or we should consider a different approach.

@lrhn
Copy link
Member

lrhn commented Feb 11, 2025

Now that I write it down, I'm realizing maybe the fact that we have any non-final fields in any of the parameters means that the class cannot ever be instantiated at compile time as a constant? And therefore marking such class as const is silly in itself?

That may be true. If the parameter is not nullable, and the parameter class is unlikely to ever get a constant-capable subclass (unlike fx List which looks mutable, but isn't always), then the constructor probably doesn't need to be const.

I don't recommend making things const just because you can. If there is no expected use-case for creating a const instance, then it's safer to not expose a const constructor.
You can always add one later, but it's a breaking change to remove a const.

@TekExplorer
Copy link

You know, the first thing that comes to mind here is actually overloading.

class Foo {
  const Foo(this.bar);
        Foo(this.bar) : assert(bar.whatever != null);
}

Well, that and constant/stable accessors.

honestly though, in this case, you probably cant use const here anyway as-is.

perhaps you should focus more on making sure that the source object is pre-validated, such as by having the argument be a variant where whatever is non-nullable, then you could safely use const, since its already valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

3 participants