For reasons beyond logic, null matches any exception type.

For reasons beyond logic, null matches any exception type.

Committed: https://code.google.com/p/dart/source/detail?r=5017

[ahe, 2012-02-29 20:44:20]

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
File dart/frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
dart/frog/leg/ssa/codegen.dart:934: buffer.add(' === (void 0) ||');
I think this will work well for checked mode as well.

[ngeoffray, 2012-03-01 08:31:16]

LGTM

[Lasse Reichstein Nielsen, 2012-03-01 08:38:17]

DBC

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
File dart/frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
dart/frog/leg/ssa/codegen.dart:933: use(input,
JSPrecedence.LOGICAL_OR_PRECEDENCE);
That should be EQUALITY_PRECEDENCE. It's the left hand side of an equality
comparison.

[Lasse Reichstein Nielsen, 2012-03-02 09:44:09]

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
File dart/frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
dart/frog/leg/ssa/codegen.dart:935: }
This also changes the expected precedence for the following code.
The check* methods are safe, since they generate === operators, but the && and
|| expressions below uses beginExpression relative to the outer precedence, not
the precedence of the || inserted above. 
I think we need some support for executing code in a different precedence scope.

[ahe, 2012-03-06 10:16:01]

Hi Lasse and Nicolas,

Thank you for taking a look.

Cheers,
Peter

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
File dart/frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
dart/frog/leg/ssa/codegen.dart:933: use(input,
JSPrecedence.LOGICAL_OR_PRECEDENCE);
On 2012/03/01 08:38:17, Lasse Reichstein Nielsen wrote:
> That should be EQUALITY_PRECEDENCE. It's the left hand side of an equality
> comparison.

Done.

https://chromiumcodereview.appspot.com/9549002/diff/1/dart/frog/leg/ssa/codeg...
dart/frog/leg/ssa/codegen.dart:935: }
On 2012/03/02 09:44:09, Lasse Reichstein Nielsen wrote:
> This also changes the expected precedence for the following code.
> The check* methods are safe, since they generate === operators, but the && and
> || expressions below uses beginExpression relative to the outer precedence,
not
> the precedence of the || inserted above. 
> I think we need some support for executing code in a different precedence
scope.

Done.

[Lasse Reichstein Nielsen, 2012-03-06 10:31:08]

https://chromiumcodereview.appspot.com/9549002/diff/7001/dart/frog/leg/ssa/co...
File dart/frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9549002/diff/7001/dart/frog/leg/ssa/co...
dart/frog/leg/ssa/codegen.dart:935:
beginExpression(JSPrecedence.LOGICAL_OR_PRECEDENCE);
Sadly, that won't do what you want. It doesn't change the expectations of the
following code, but merely puts a start parenthesis if the current expectation
is harder binding than a ||.

We can either create a function withPrecedence(precedence, thunk) that sets the
expected precedence during execution of  the thunk, or we can manually update
expectedPrecedence here (if it's the only place we need it, the extra
abstraction might be overdoing it).

I.e.:

int oldPrecedence = currentPrecedence;
if (node.nullOK) {
  ...
  currentPrecedence = LOGICAL_OR_PRECEDENCE;
}
... expression or RHS of || ...
if (node.nullOk) {
  currentPrecedence = oldPrecedence;
}

It breaks the abstraction a little, since we don't manipulate currentPrecedence
anywhere else outside of use/visit, but if it's only one place it should be
fine.

[ahe, 2012-03-06 10:57:38]

https://chromiumcodereview.appspot.com/9549002/diff/7001/dart/frog/leg/ssa/co...
File dart/frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9549002/diff/7001/dart/frog/leg/ssa/co...
dart/frog/leg/ssa/codegen.dart:935:
beginExpression(JSPrecedence.LOGICAL_OR_PRECEDENCE);
On 2012/03/06 10:31:08, Lasse Reichstein Nielsen wrote:
> Sadly, that won't do what you want. It doesn't change the expectations of the
> following code, but merely puts a start parenthesis if the current expectation
> is harder binding than a ||.
> 
> We can either create a function withPrecedence(precedence, thunk) that sets
the
> expected precedence during execution of  the thunk, or we can manually update
> expectedPrecedence here (if it's the only place we need it, the extra
> abstraction might be overdoing it).
> 
> I.e.:
> 
> int oldPrecedence = currentPrecedence;
> if (node.nullOK) {
>   ...
>   currentPrecedence = LOGICAL_OR_PRECEDENCE;
> }
> ... expression or RHS of || ...
> if (node.nullOk) {
>   currentPrecedence = oldPrecedence;
> }
> 
> It breaks the abstraction a little, since we don't manipulate
currentPrecedence
> anywhere else outside of use/visit, but if it's only one place it should be
> fine.

I think this is way too complicated, and I have no idea how to apply your
suggestion. The mental model behind this is apparently not based on building up
expressions from subexpressions. As long as that is not the case, this will be
an endless source of problems.

I think it was a mistake to add the precedence in this form. We would be better
off wrapping everything in parentheses.

I'll add a pair of parentheses.