Proposed spec for DEP for assert with messages.

docs/language/dartLangSpec.tex

Index: docs/language/dartLangSpec.tex
diff --git a/docs/language/dartLangSpec.tex b/docs/language/dartLangSpec.tex
index 3850f200dfcfc00dea26f2a7b80fdf71a2c7f3a8..cda6eb0cb79b7836862b895668a68655ea40c53c 100644
--- a/docs/language/dartLangSpec.tex
+++ b/docs/language/dartLangSpec.tex
@@ -6457,24 +6457,34 @@ An {\em assert statement} is used to disrupt normal execution if a given boolean
 
 \begin{grammar}
 {\bf assertStatement:}
-   assert `(' conditionalExpression `)' `{\escapegrammar ;}'
+   assert `(' conditionalExpression (`,' expression)?  `)' `{\escapegrammar ;}'
       .
 \end{grammar}
       
 \LMHash{}
-The assert statement has no effect in production mode. In checked mode, execution of an assert statement \code{\ASSERT{}($e$);} proceeds as follows:
+The assert statement has no effect in production mode. In checked mode, execution of an assert statement \code{\ASSERT{}($e$);} or \code{\ASSERT{}($e$, $s$);} proceeds as follows:
 
 \LMHash{}
 The conditional expression $e$ is evaluated to an object $o$. If the class of $o$ is a subtype of \code{Function} then let $r$ be the result of invoking $o$ with no arguments. Otherwise, let $r$ be $o$. 
-It is a dynamic type error if $o$ is not of type \code{bool} or of type \code{Function}, or if $r$ is not of type \code{bool}.  If $r$ is \FALSE{}, we say that the assertion failed. If $r$ is \TRUE{}, we say that the assertion succeeded. If the assertion succeeded, execution of the assert statement is complete. If the assertion failed, an \code{AssertionError} is thrown.
+It is a dynamic type error if $o$ is not of type \code{bool} or of type \code{Function}, or if $r$ is not of type \code{bool}.  If $r$ is \FALSE{}, we say that the assertion failed. If $r$ is \TRUE{}, we say that the assertion succeeded. If the assertion succeeded, execution of the assert statement is complete. 
+
+If the assertion failed, and the assertion is of the form \code{\ASSERT{}($e$);} then the expression \cd{\THROW{} \NEW{} AssertionError()} is evaluated.
+ If the assertion failed and the assertion is of the form \code{\ASSERT{}($e$, $s$);} then the expression \cd{\THROW{} \NEW{} AssertionError(s)} is evaluated.

[Lasse Reichstein Nielsen, 2015/09/18 12:26:28]

I'd recommend against specifying the exact constructor used to create the
AssertionError.

We can't change the AssertionError constructor to a factory at this point
(someone may subclass AssertioneError), so an "AssertionError(s)" constructor
will be generative and have to store the parameter on the object.

Since TypeError extends AssertionError (and we can't change that now, that would
also be a breaking change), any message that we put on the AssertionError class
will be carried over to the TypeError class where it won't make sense. 

If we don't specify the exact constructor to use here, just that it generates an
object that implements AssertionError, then we can use any constructor and
subclass for it - which implementations already do for AssertionError().

Also, you are saying that "throw new AssertionError()" is evaluated, however
that expression is *not part of the program*. When throwing, we capture a stack
trace, but what is the stack trace for code that doesn't have a file or
location? Generally, I'm concerned when non-existing code is explicitly
evaluated. It would at least be better if it the semantics was written as doing
something similar to that expression instead of *being* that expression.

+
+\rationale {
+The optional argument $s$ is intended to allow a suitable message to be associated with the assertion. An implementation should endeavour to display the message in a manner that is useful to the developer.
+}
+\commentary{
+The definition above implies that if $s$ does not evaluate to a \cd{String}, a dynamic error is thrown, because the argument to the constructor of \cd{AssertionError} is typed as \cd{String} and we are running in checked mode. It also implies that $s$ is not evaluated if the assertion succeeded, and that any exception that occurs during evaluation of $s$ is thrown and propagated as usual, in which case the assertion error is lost.

[sra1, 2015/09/17 19:55:22]

Where is it specified that the argument to AssertionError's constructor is typed
as String?

[Lasse Reichstein Nielsen, 2015/10/15 09:14:48]

This means that a type error in the assertion message will hide the real
assertion error - which is always more important to the user.

We should not throw in the message if it can be avoided. If anything, we should
actively catch errors in the message expression to prevent them from causing the
real assertion error to be lost.

[Lasse Reichstein Nielsen, 2015/10/15 09:14:48]

I guess that's implicitly specified by this commentary.

+}
 
 %\Q{Might be cleaner to define it as \code{if (!$e$) \{\THROW{} \NEW{} AssertionError();\}} (in checked mode only).
 %What about an error message as part of the assert?}
 
 \LMHash{}
- It is a static type warning if the type of $e$ may not be assigned to either  \code{bool} or $() \rightarrow$ \code{bool}.  
+ It is a static type warning if the type of $e$ may not be assigned to either  \code{bool} or $() \rightarrow$ \code{bool}.  It is a static type warning if the type of $s$ may not be assigned to \cd{String}.

[sra1, 2015/09/17 19:55:22]

I think it is a mistake to constrain the message to String.

This will encourage the user to write expressions to generate strings where it
is not really necessary.

Compare:

    assert(age >= 10, '$age is too young');

    assert(age >= 10, '$age');

    assert(age >= 10, age);


The proposal forces the first two, but this is a worse user experience in almost
every way:

1. Less concise.

2. The risk that toString() throws a confounding error, obfuscating the true
problem.

3. The user has to worry about safely generating strings at every assert rather
than have the system take care of it in one place.

4. The string generating expressions take up space in the compiled
representation of the program (e.g. JavaScript)

5. The string generating expressions are largely untestable.

It is often *impossible* to test the assertion messages.  If the assertion
condition expresses a deep invariant then the message is unreachable!  
A disproportionate number of defects are on error paths. Lets help the user not
add more by letting them assert with general values.

 
-\rationale{Why is this a statement, not a built in function call? Because it is handled magically so it has no effect and no overhead in production mode. Also, in the absence of final methods. one could not prevent it being overridden (though there is no real harm in that).  It cannot be viewed as a function call that is being optimized away because the argument might have side effects.
+\rationale{Why is this a statement, not a built in function call? Because it is handled magically so it has no effect and no overhead in production mode. Also, in the absence of final methods. one could not prevent it being overridden (though there is no real harm in that).  It cannot be viewed as a function call that is being optimized away because the argument might have side effects. And lastly, the optional string message would have to be evaluated in all cases if \ASSERT{} were a function.
 }
 
 %If a lexically visible declaration named \code{assert} is in scope, an assert statement