Skip to content

refactor: Parser should take a string argument#5832

Merged
9rnsr merged 1 commit intodlang:masterfrom
WalterBright:parser
Jun 3, 2016
Merged

refactor: Parser should take a string argument#5832
9rnsr merged 1 commit intodlang:masterfrom
WalterBright:parser

Conversation

@WalterBright
Copy link
Member

Because strlen() must die.

@WalterBright WalterBright changed the title Parser should take a string argument refactor: Parser should take a string argument Jun 3, 2016
@WalterBright WalterBright force-pushed the parser branch 2 times, most recently from 7bbd613 to ecb3dbe Compare June 3, 2016 05:42
@9rnsr
Copy link
Contributor

9rnsr commented Jun 3, 2016

Auto-merge toggled on

* allocated by mem.xmalloc().
*/
final const(char)* toStringz() const
extern (D) final const(char)[] toStringz() const
Copy link
Contributor

Choose a reason for hiding this comment

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

const char *toStringz() const;

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, what's expected ? Should we just ignore the C++ side, or remove it (or something else) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

In Walter's mind, the mismatch between D and C++ code is less important issue than frustration about code style.
Refer: #5751.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's never called from C++ code anyway, so I just removed it from expression.h

Copy link
Contributor

Choose a reason for hiding this comment

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

Please be really careful when doing this with virtual functions. The C++ and D sides do not share vtables, so it might not get noticed immediately.

@9rnsr 9rnsr merged commit bc9d23c into dlang:master Jun 3, 2016
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.

4 participants