Skip to content

std.xml: make work with -dip1000#5387

Merged
dlang-bot merged 1 commit intodlang:masterfrom
WalterBright:xml-scope
Jun 10, 2017
Merged

std.xml: make work with -dip1000#5387
dlang-bot merged 1 commit intodlang:masterfrom
WalterBright:xml-scope

Conversation

@WalterBright
Copy link
Copy Markdown
Member

Add annotations to make its interaction with Object safe, and scope to make it work with -dip1000.

It requires covariant overrides to work, so it is blocked by dlang/dmd#6731

override string toString() scope const @safe pure nothrow { return "<!--" ~ content ~ "-->"; }

override @property @safe @nogc pure nothrow bool isEmptyXML() const { return false; } /// Returns false always
override @property @safe @nogc pure nothrow scope bool isEmptyXML() const { return false; } /// Returns false always
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is getting out of hand

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm... I'm curious why this is necessary, as this doesn't return any references and is pure?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is getting out of hand

The whole std.xml needs to go anyway. If it was a templated object, most of these attributes would simply be inferred.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is a very simple solution: just write it as isEmptyXML()() instead, and boom, you have compiler attribute inference.

(Though I understand that this doesn't always work all of the time, but it's worth a try.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh wait, this is an override. Nevermind what I said. :-(

Copy link
Copy Markdown
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

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

A lot of this code is not covered. Not this PR's fault though.

@dlang-bot dlang-bot merged commit 6f57e01 into dlang:master Jun 10, 2017
@UplinkCoder
Copy link
Copy Markdown
Member

UplinkCoder commented Jun 11, 2017

the released version of druntime is incompatible with this.
function std.xml.Item.opEquals does not override any function, did you mean to override 'object.Object.opEquals'?

@CyberShadow
Copy link
Copy Markdown
Member

the released version of druntime is incompatible with this

I don't think we need to worry about that, dmd/druntime/phobos are developed in sync and there is no expectation that mixing versions works.

@wilzbach
Copy link
Copy Markdown
Contributor

I don't think we need to worry about that, dmd/druntime/phobos are developed in sync and there is no expectation that mixing versions works.

Agreed and FWIW Phobos-dev is almost never works compatible with the released version - just try to compile a normal D file with a few imports to Phobos in the Phobos master directory ;-)

@UplinkCoder
Copy link
Copy Markdown
Member

druntime ~master is also incompatible.

@CyberShadow
Copy link
Copy Markdown
Member

If that were so then all CI would be broken, so look for the problem elsewhere.

@wilzbach
Copy link
Copy Markdown
Contributor

If that were so then all CI would be broken, so look for the problem elsewhere.

Have you cleaned dmd, druntime and Phobos and built from scratch?

@UplinkCoder
Copy link
Copy Markdown
Member

EDIT:
it was a problelem with newCTFE not having the scope covarriance patch.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants