assignment_to_final_local interaction with try/catch

[Hixie]

In the following code, x can only be set once, but the second assignment is considered an error.

void main() {
  final int x;
  try {
    // thow 'test';
    x = 1;
  } catch {
    x = 2; // The final variable 'x' can only be set once.
  }
  print(x);
}

I think if the assignment is the last statement in a try block, it should not be considered to conflict with an assignment in a catch block.

[bwilkerson]

@stereotype441 Is this handled by the definite assignment analysis?

[stereotype441]

Yes, this is handled by definite assignment analysis (or, rather, it's handled by flow analysis, which combines definite assignment analysis with type promotion and reachability analysis). Flow analysis assumes that an exception could occur at any time during a try block, even before its first statement or after its last statement.

I think that in principle it would be safe to make the change @Hixie suggests above, but I'm not 100% certain. Are we positive that neither OutOfMemoryError nor StackOverflowError could happen after the assignment? Do we care? Generally where issues of soundness are concerned, I like to be extra-cautious, and a lot of the logic of flow analysis affects soundness (although maybe this particular aspect of it doesn't).

In any case, I can definitely see the appeal of making the suggested change. Although it would be more complex than what we do today, one of the guiding principles I've tried to follow in the design of flow analysis is to remember that complexity in the analysis isn't necessarily bad, because the user doesn't notice the complexity of flow analysis; what the user notices is when flow analysis comes to a different conclusion than their own intuitive mental model of their code's behaviour. And their intuitive mental model of their code's behaviour is a very sophisticated one (it must be, otherwise they wouldn't be able to do their job as a programmer). So from that standpoint I would support making this change.

On the other hand, it would be a non-trivial amount of work to support this, because of the two-phase design of flow analysis. The information that leads us to conclude that the two assignments to x might conflict is coming in part from data we gather about the try-block during phase one. Phase one happens during parsing (at least in the front end it does), so at the time that we register the variable as being assigned-to during the try-block, we haven't yet seen the }, so we don't know whether the assignment is the last statement in the block or not. Obviously we could re-architect things to get around that restriction, but it would make the change a lot less trivial than it might seem at first.

Another principle I've tried to follow in the design of flow analysis is to try to prevent the results of flow analysis from being changed by the addition or removal of "unrelated code". For example, I took extra care to ensure that adding a throw at the top of a function wouldn't break promotions later in the function that are based on reachability, because if someone wants to quickly hack in a throw as a temporary debugging measure, I didn't want that to trigger compile errors in a later part of the function that is now unreachable. Similarly, I took extra care to make sure that it's always safe to add logging code, or calls to print, anywhere in a function, without changing the meaning of other things that are happening in the function. If we decide to let the last statement of a try-block be special, then we would break that, because then a user wouldn't be able to tack a print statement on the end of a try-block without potentially causing a compile-time error.

[Hixie]

I definitely don't think this is a high priority.

My naive assumption is that this is safe because it would only apply when you're setting a local variable, and there's no memory to be allocated, and no code to be called, during an assignment to a local variable. (This logic wouldn't apply to other assignments, but they're not affected by this lint/warning/whatever anyway.)

I agree that it's unfortunate that the behaviour would change when you add a print after the assignment at the end of a try block.

[julemand101]

@jarrodcolburn The finally block will be executed regardless of any error, so your variable assignment in the finally block should definitely be an error.

[lrhn]

Indeed. There is no finally block in the original example, just a catch block which forgot the variable.

The point is that the body cannot throw after assigning to the variable, so the catch block won't be reached in a state where the variable is assigned.

That'd be guaranteed by the assignment being the last operation, and the assignment itself not being able to throw after assigning (which is generally true for local variables - even late ones cannot throw on an assignment if they weren't already assigned before, at which point we're outside the scope of this example).

That is the simpler version of "All code executed after the assignment is unable to throw". Dart generally don't assume that anything is unable to throw, so "zero code" is the only real option. Which is also why addressing this would be so specialized that it's probably not worth the effort.