Skip to content

ImportC: add __check(assign-expression) extension#14026

Merged
RazvanN7 merged 1 commit intodlang:masterfrom
WalterBright:__assert
Feb 7, 2023
Merged

ImportC: add __check(assign-expression) extension#14026
RazvanN7 merged 1 commit intodlang:masterfrom
WalterBright:__assert

Conversation

@WalterBright
Copy link
Member

Trying to use assert in ImportC without the preprocessor gets tiresome, so I added __assert as an extension.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14026"

@WalterBright WalterBright force-pushed the __assert branch 3 times, most recently from 2ccdcdf to de5ac77 Compare April 26, 2022 07:20
@huglovefan
Copy link
Contributor

the name __assert is already used for the assert failure function on some platforms (see druntime core.stdc.assert for a list)

if it's made a keyword, the assert header and uses of the macro will fail to parse

$ cpp /usr/include/assert.h > assert.i
$ dmd -verrors=context assert.i
/usr/include/assert.h(81): Error: identifier or `(` expected
extern void __assert (const char *__assertion, const char *__file, int __line)
            ^

@WalterBright
Copy link
Member Author

Curses! Any ideas for a replacement keyword for __assert ?

@PetarKirov
Copy link
Member

How about _Assert?

@WalterBright WalterBright changed the title ImportC: add __assert(assign-expression) extension ImportC: add __check(assign-expression) extension May 12, 2022
@WalterBright
Copy link
Member Author

Changed __assert to __check

@WalterBright WalterBright force-pushed the __assert branch 4 times, most recently from 89e2df8 to 3c45c88 Compare May 14, 2022 07:58
@ljmf00
Copy link
Member

ljmf00 commented May 14, 2022

Changed __assert to __check

I already dumped my thoughts on using known keywords for reserved identifiers in C when __import got merged. __ is reserved and we better have a tiny prefix to avoid such conflicts, but well we got to deal with it someday.

src/dmd/cparse.d Outdated
e = cparseGenericSelection();
break;

case TOK._assert: // __assert(assign-exp) extension
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be renamed to __check as well

@RazvanN7
Copy link
Contributor

ping @WalterBright

@salihdb
Copy link

salihdb commented Jul 9, 2022

I want to make a new suggestion to you; if the English meaning is not contradictory!

"ascertain" that can't introduce new conflicts never?

  assert(true);
  ascertain(true);

Sounds good, huh? Okay, it's 3 letters more but it is better than underscore typed by double key press.:)

SDB@79

@RazvanN7
Copy link
Contributor

@WalterBright what is the status of this PR?

@RazvanN7
Copy link
Contributor

RazvanN7 commented Sep 1, 2022

@WalterBright is this good to go?

ibuclaw
ibuclaw previously requested changes Sep 1, 2022
Copy link
Member

@ibuclaw ibuclaw left a comment

Choose a reason for hiding this comment

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

Looked at the documentation, looked at the first source file. Definitely not ready to go.

@WalterBright WalterBright dismissed ibuclaw’s stale review February 7, 2023 05:36

I guessed at what you meant.

@WalterBright WalterBright added the Review:Ready To Merge Let's get this merged label Feb 7, 2023
@WalterBright
Copy link
Member Author

Ready to merge

@RazvanN7 RazvanN7 merged commit 300ba0c into dlang:master Feb 7, 2023
@ibuclaw
Copy link
Member

ibuclaw commented Feb 16, 2023

@WalterBright @RazvanN7 How do we know this works? The test is never ran by the testsuite!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

9 participants

Comments