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

Specify that constant assertions are always evaluated #1271

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lrhn
Copy link
Member

@lrhn lrhn commented Oct 27, 2020

See #447 for context.

@lrhn lrhn requested review from leafpetersen and eernstg October 27, 2020 14:28
@google-cla google-cla bot added the cla: yes label Oct 27, 2020
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I put the question about going ahead and doing this on the language team meeting agenda for tomorrow.


Currently assertions are enabled only when the compiler is passed an `enable-asserts` flag.
With null safety, when an assert is evaluated as part of a constant invocation of a `const`
constructor, any `assert`s in the initializer list are evaluated.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could say are enabled, to keep the number of concepts about assertions lower, and allowing implementations to compile non-enabled assertions away entirely, or keep them and skip them, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that sentence is garbled. That said, an assertion "being enabled" is a concept more than simply saying what its semantics are in different cases. Implementations can still completely remove run-time assertions from the compiled output when assertions are disabled, because they have the "null-semantics": completes normally with no side effects.

### Constant assertions

Currently assertions are enabled only when the compiler is passed an `enable-asserts` flag.
With null safety, when an assert is evaluated as part of a constant invocation of a `const`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could call them an assertion as a rule, and only use assert when referring to actual code. The language specification has assert statement which is a statement that is an assertion, but otherwise it refers to assertions as assertions. Might as well do that here, too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I think I did that deliberately when I introduced assertions into initializer lists (where they are not statements).

More precisely, a assert _s_ of the form <code>assert(_e_<sub>1</sub>, _e_<sub>2</sub>)</code>
is executed as follows:
* If the execution of _s_ happens during evaluation of a const invocation of the
surrounding `const` constructor, then _e_<sub>1</sub> is evaluated to a constant value _c_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about then it is a compile-time error if _e_<sub>1</sub> is not a constant expression with the value true``?

Then we don't have to mention 'completes normally' as if it's part of the constant evaluation semantics.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. We already do the "compile-time error if expression would throw" for constant evaluation.

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could rely more on the existing specification text, and just have a very short section in this document.

@@ -1008,6 +1011,33 @@ void main() {
}
```

### Constant assertions

Currently assertions are enabled only when the compiler is passed an `enable-asserts` flag.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text in section 'Assert' of the language specification already says almost everything which is said here.

Wouldn't it suffice to have a very short section here? We would then rely on the existing specification text for all the details:

Suggested change
Currently assertions are enabled only when the compiler is passed an `enable-asserts` flag.
Assertions are enabled during the evaluation of a constant object expression.
*This means that properties checked by assertions will always be enforced with constant expressions. The implementation specific mechanism to enable assertions is used to enable the remaining assertions, namely the ones that are executed at run time.*

Copy link
Member Author

@lrhn lrhn Oct 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tricky, because we don't actually separate compile-time and run-time in the specification, we just specify the meaning of constants such that they can be pre-computed, but we don't actually say that you have to do so.

We still need a way to specify that something is happening as part of constant evaluation because it matters in some situations. For example const x = 1 ~/ 0; is a compile-time error instead of throwing a DivisionByNullError ... and assert(false, e2) shouldn't need to evaluate e2, so we don't need to require that e2 is a potentially constant expression.

Which actually does mean that we can't reasonably delay evaluation of constant expressions until run-time unless we allow compile-time errors at run-time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, we specify that evaluation of a constant object expression occurs at run time and then it is (recursively, bottom-up) canonicalized. It makes no difference whether that constant object expression is in a constant context, whether it is used in the initialization of a const variable, etc., it is a rule for <constObjectExpression> anywhere. Of course, if it is in a constant context then the const keyword may be absent, it is still statically known to be there.

This rule is compatible with an evaluation at compile time. However, it doesn't require an implementation to perform the evaluation at compile time and reuse that result every time it is needed at run time. For instance, the expression could be evaluated at compile time and the result could be discarded, and then evaluation and canonicalization could be done at run time. Also, because the constant sublanguage is small and Turing-incomplete, execution is guaranteed to be possible at compile time. So when we want to determine whether an error should be emitted, we can evaluate the constant object expression.

So if we do say 'Assertions are enabled during the evaluation of a constant object expression' then it is allowed for an implementation to raise the compile-time error early ("at compile time"). That's probably exactly what we want.

This also means that if a tool gives rise to an execution where a constant object expression is actually evaluated at run time then there will be a run-time error, even though we set out to say that these errors should occur at compile-time. But tools don't have to do this, so in practice we can maintain that these errors occur early.

If an execution uses hot reload then compilation will occur at run time anyway.

Don't you think that would be OK for tool teams to work with?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants