Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

add core.stdc.assert_ #1839

Merged
merged 1 commit into from
Jul 1, 2017
Merged

add core.stdc.assert_ #1839

merged 1 commit into from
Jul 1, 2017

Conversation

WalterBright
Copy link
Member

For those who would want to call the C assert function rather than the druntime one.

@WalterBright WalterBright force-pushed the assert_ branch 2 times, most recently from d4d6b84 to e5348ec Compare June 15, 2017 06:02
win64.mak Outdated
DFLAGS=-m$(MODEL) -conf= -O -release -dip1000 -inline -w -Isrc -Iimport
UDFLAGS=-m$(MODEL) -conf= -O -release -dip1000 -w -Isrc -Iimport
DFLAGS=-m$(MODEL) -conf= -O -release -dip1000 -dip1008 -inline -w -Isrc -Iimport
UDFLAGS=-m$(MODEL) -conf= -O -release -dip1000 -dip1008 -w -Isrc -Iimport
Copy link
Member

Choose a reason for hiding this comment

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

Adding -dip1008 seems unrelated. There are also a couple more unrelated changes to win64.mak that cause build failures.

}
else version (CRuntime_Microsoft)
{
void _assert(const(char)* exp, const(char)* file, uint line);
Copy link
Member

Choose a reason for hiding this comment

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

There is no _assert in the MS runtime, they use _wassert with wide strings only:

void __cdecl _wassert(const wchar_t * _Message, const wchar_t *_File, unsigned _Line);

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, there is. It just isn't in the .h file.

Copy link
Member

Choose a reason for hiding this comment

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

I see. A comment would be nice for the next time I stumble over this.

Should we add the _wassert version, too, similar to the various declaration on the other platforms?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

@rainers
Copy link
Member

rainers commented Jun 15, 2017

Not a blocker, but the documentation only lists the glibc version declarations. This can be pretty confusing for other platforms. The ddox documentation doesn't list any function at all.

extern (C):
@trusted:
nothrow:
@nogc:
Copy link
Member

Choose a reason for hiding this comment

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

pure?

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'm not sure that a function never returning means it is pure.

Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand, this provides an alternative to built-in assert. Since the built-in assert is usable from pure functions, I think this should also be.

I'm not sure that a function never returning means it is pure.

That should never happen in correct programs. But in general pure functions are allowed to terminate the program, according to the language spec. So I think the C assert functions fit the bill.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @ZombineDev, should be pure. The betterC purists will come back at you anyway without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pure functions that return void can be elided by the compiler.

@rainers
Copy link
Member

rainers commented Jun 20, 2017

I think this would be more useful with a template c_assert or similar that translates the C macro assert for the different platforms.

@WalterBright
Copy link
Member Author

I think this would be more useful with a template c_assert or similar that translates the C macro assert for the different platforms.

The purpose of these imports is not to fix the interface, but to simply declare it.

@rainers
Copy link
Member

rainers commented Jun 22, 2017

Pure functions that return void can be elided by the compiler.

If these functions are actually elided, then we'll have to go without pure for now. I guess we need a solution for this, though. There are already unintuitive hacks in druntime to work around this, e.g. onOutOfMemoryError.

@rainers
Copy link
Member

rainers commented Jun 22, 2017

The purpose of these imports is not to fix the interface, but to simply declare it.

The C interface is assert, though, not __assert_rtn. Other core.stdc files translate the macros, too. See https://github.com/dlang/druntime/blob/master/src/core/stdc/stdio.d#L1068 for an example.

@WalterBright
Copy link
Member Author

I guess we need a solution for this, though.

The solution is a noreturn function attribute of some sort.

@WalterBright
Copy link
Member Author

The C interface is assert, though, not __assert_rtn.

We also declare the internal functions in the core.stdc.* imports.

@rainers
Copy link
Member

rainers commented Jun 23, 2017

We also declare the internal functions in the core.stdc.* imports.

Sure, I never said these should be removed. But without an implementation of the C assert macro, everyone will have to implement their own version forwarding to the platform specific functions.

@rainers
Copy link
Member

rainers commented Jun 23, 2017

The solution is a noreturn function attribute of some sort.

This will also need a mightnotreturn attribute if the termination is conditional.

@WalterBright
Copy link
Member Author

This will also need a mightnotreturn attribute if the termination is conditional.

Every function is mightnotreturn. It's OT anyway.

@rainers
Copy link
Member

rainers commented Jul 1, 2017

Every function is mightnotreturn. It's OT anyway.

If that's true you must not elide any pure function if you want to keep the side effect of noreturn pure calls.

Copy link
Member

@rainers rainers left a comment

Choose a reason for hiding this comment

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

Let's move forward with this. A C assert wrapper can be added later if the need pops up. We have lived without it for years.

@dlang-bot dlang-bot merged commit b6d5b74 into dlang:master Jul 1, 2017
@WalterBright WalterBright deleted the assert_ branch July 1, 2017 21:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants