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

Improve assert expression documentation. #624

Merged
3 commits merged into from
Oct 2, 2014
Merged

Conversation

quickfur
Copy link
Member

@quickfur quickfur commented Aug 1, 2014

Per discussion on forum.

@quickfur
Copy link
Member Author

quickfur commented Aug 1, 2014

@DanielGibson
Copy link

It's also unclear when AssertError is thrown and when some HALT instruction is executed.
Maybe, while at it, this could also be documented more clearly.
Also remember that assert is documented both in expressions and in the "Contract Programming" section.

@quickfur
Copy link
Member Author

quickfur commented Aug 1, 2014

AFAIK, assert(0) will always cause the program to halt, no? I'm not sure if distinguishing between HLT and throwing AssertError is that big of an issue, because you aren't supposed to catch Errors anyway. Or if you do, you should only do emergency cleanup and not attempt to recover, because your program is already in an invalid state -- Errors can be thrown from inside nothrow functions, for example, so scope(exit) blocks may not have run, dtors may not have been invoked, etc. Basically, they represent fatal, unrecoverable problems with the program. In practice, if AssertError is thrown, your program is bound to terminate, so I'm not sure it's that much different from HLT. It might be a good idea to give compiler implementors the option to choose between them (on some platforms it might be more practical to just HLT immediately on assertion failure, for example, rather than performing a potentially expensive stack unwinding).

@quickfur
Copy link
Member Author

quickfur commented Aug 1, 2014

Web preview for contracts page: http://eusebeia.dyndns.org/~hsteoh/tmp/web/contracts


$(P It is an error if the $(I expression) contains any side effects
that the program depends on. The compiler may optionally not evaluate
assert expressions at all.)
Copy link
Member

Choose a reason for hiding this comment

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

Replace with:

$(P It is implementation defined whether the $(I expression) is evaluated at run time or not. Programs that rely on side effects of $(I expression) are invalid.)

@tgehr
Copy link
Contributor

tgehr commented Aug 1, 2014

The documentation of '-release' and version(assert) need to be updated accordingly as well.

verify that the expression is indeed true. If it is false, an $(D
AssertError) is thrown.)

$(P When compiling for release, this check is not generated. The
Copy link
Contributor

Choose a reason for hiding this comment

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

It is irrelevant to claim it isn't generated. It's undefined behaviour. Undefined behaviour means, it is (maybe unlikely, maybe not what DMD does now, but) theoretically possible that the check is actually generated.

Copy link
Member

Choose a reason for hiding this comment

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

@tgehr If we write it here in the spec, it's no longer undefined.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Hackerpilot: Undefined behaviour has a precise technical meaning. It overrides anything else you might say about the program, not the other way around. You might be confusing undefined behaviour and unspecified behaviour.

@DanielGibson
Copy link

Am 01.08.2014 21:42, schrieb H. S. Teoh:

AFAIK, |assert(0)| will /always/ cause the program to halt, no? I'm not
sure if distinguishing between |HLT| and throwing |AssertError| is that
big of an issue, because you aren't supposed to catch Errors anyway.

I'm not talking about the assert(0) special case, but about assert() in
general.
If there was no need to distinguish, AssertError didn't exist.
So I'm sure there is some rule when HLT is executed and when that error
is thrown and it should be documented.

@quickfur
Copy link
Member Author

quickfur commented Aug 1, 2014

I think Walter's intention is to say that any program that has a failed assertion is invalid by definition, and so its behaviour is undefined. Things like AssertError and HLT are implementation details that the compiler may offer as help for debugging these failed assertions, but one shouldn't be relying on such things being present in the program.

@quickfur
Copy link
Member Author

quickfur commented Aug 1, 2014

Updated web previews.

@quickfur
Copy link
Member Author

quickfur commented Aug 1, 2014

@tgehr Where is the documentation for -release? I can't seem to find the page. Do you know the filename?

@tgehr
Copy link
Contributor

tgehr commented Aug 1, 2014

It is in this and related files linked from there:
http://dlang.org/dmd-linux.html

@quickfur
Copy link
Member Author

quickfur commented Aug 2, 2014

Update web previews again.

$(I must) uphold. Any failure of this expression represents a logic
error in the code that must be fixed. A program for which the assert
contract is false is, by definition, invalid, and therefore has
undefined behaviour past that point.)
Copy link
Contributor

Choose a reason for hiding this comment

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

"past that point" is misleadingly implying that the code before that point won't be affected. If an optimizer can prove that undefined behaviour will occur after some code has executed (in a certain way), it might scrub (transform) that code based on this fact.

@tgehr
Copy link
Contributor

tgehr commented Aug 2, 2014

There is no treatment of the issue that @safe code will possibly become memory-unsafe if it contains assertions.


$(P It is implementation defined whether the $(I expression) is
evaluated at run time or not. Programs that rely on side effects of $(I
expression) are invalid.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: consistently use either the expression or just $(I expression).

@quickfur
Copy link
Member Author

quickfur commented Aug 5, 2014

@tgehr Where would such treatment be documented, if we were to document it? I'm not sure why we're fixating on asserts in @safe code, because any other language construct that results in undefined behaviour will, by definition, also risk breaking the memory safety of @safe. I believe Walter has stated before that @safe just means it restricts memory-unsafe operations, but it wasn't designed to eliminate undefined behaviour. At least, that's how I understand it.

I think it's really up to Walter how he wants this documented.

@tgehr
Copy link
Contributor

tgehr commented Aug 5, 2014

http://dlang.org/safed.html
Quote: "SafeD is easy to learn and it keeps the programmers away from undefined behaviors."

@quickfur
Copy link
Member Author

quickfur commented Aug 7, 2014

I think that page needs to be fixed. Walter & Andrei have repeatedly made it clear that @safe means "no memory corruption", not "no undefined behaviour".

@tgehr
Copy link
Contributor

tgehr commented Aug 7, 2014

No memory corruption implies no undefined behaviour.

@quickfur
Copy link
Member Author

quickfur commented Aug 7, 2014

I don't follow. Integer overflow is undefined behaviour, but there is no memory corruption involved.

@tgehr
Copy link
Contributor

tgehr commented Aug 7, 2014

If the program can behave in any way, it can in particular corrupt memory. (And I am quite confident that given enough time and inclination, one can abuse signed overflow in connection with writing into an array to trick e.g. gdc into emitting a binary that corrupts memory with the appropriate optimization flags even though main is @safe.) In any case, I don't think this is necessarily the right place to discuss this.

@ghost
Copy link

ghost commented Sep 11, 2014

Can we get an update on this? Looks nice except Walter's and Tgehr's comments.

for more information.)
$(P As a contract, an $(D assert) represents a guarantee that the code
$(I must) uphold. Any failure of this expression represents a logic
error in the code that must be fixed. A program for which the assert
Copy link
Member

Choose a reason for hiding this comment

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

This could probably benefit from saying "fixed in the source code", so it's perfectly clear that this is not fixable by catching an AssertError.

@JakobOvrum
Copy link
Member

Asserts are more interesting in @trusted code than @safe code. @safe enforces memory safety without or without asserts, but @trusted doesn't: it's easy to make the mistake of dereferencing a pointer without a runtime safety check just because the same check was done in an assert. In @trusted functions, bounds checking and other related checks must be done with enforce, not assert.

I don't follow. Integer overflow is undefined behaviour, but there is no memory corruption involved.

OT, but integer overflow is defined and expected in D, unlike in C and C++.

H. S. Teoh added 3 commits September 11, 2014 10:45
Improve assert docs on design-by-contract page too.

Improve wording based on comments.

Incorporate more suggestions from comments.

Reword per comments.

Copy-edit.

Use less ambiguous wording for version(assert) and -release.
@quickfur quickfur force-pushed the assert_doc branch 2 times, most recently from 0b385cf to 279d262 Compare September 11, 2014 17:47
@quickfur
Copy link
Member Author

Updated and squashed some commits.

@quickfur
Copy link
Member Author

ping

@quickfur
Copy link
Member Author

quickfur commented Oct 2, 2014

ping @AndrejMitrovic @9rnsr

ghost pushed a commit that referenced this pull request Oct 2, 2014
Improve assert expression documentation.
@ghost ghost merged commit cd99fd8 into dlang:master Oct 2, 2014
@ghost
Copy link

ghost commented Oct 2, 2014

Looks good, thanks for the work!

@quickfur
Copy link
Member Author

quickfur commented Oct 2, 2014

Thanks!

@quickfur quickfur deleted the assert_doc branch October 2, 2014 17:58
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants