Skip to content

[REG2.065a] Issue 12089 - std.utf.validate and inout(char[]) failts to compile #1921

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

Merged
merged 1 commit into from
Feb 10, 2014

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Feb 10, 2014

https://d.puremagic.com/issues/show_bug.cgi?id=12089

Unqual should support inout(const(T))

@monarchdodra
Copy link
Collaborator

What about bug https://d.puremagic.com/issues/show_bug.cgi?id=1308 ?

It would be nice to be able to have the initially suggested:

             static if (is(T U == const U)) alias Unqual!U Unqual;
        else static if (is(T U == immutable U)) alias Unqual!U Unqual;
        else static if (is(T U == inout U)) alias Unqual!U Unqual;
        else static if (is(T U == shared U)) alias Unqual!U Unqual;
        else alias T Unqual;

Other than that, LGTM

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 10, 2014

Sorry, I'm not sure that bug 1308 is really fixed. And, I think this change is better because it is low risk for the regression fix.

@monarchdodra
Copy link
Collaborator

Sorry, I'm not sure that bug 1308 is really fixed.

Yeah, I was just pointing out I think it's a shame to have to resort to making the full enumeration for this.

Related, doesn't ModifyTypePreservingSTC now have the exact same bug as Unqual?

And, I think this change is better because it is low risk for the regression fix.

Yup, no problem on my end. I'll merge once ModifyTypePreservingSTC is also addressed.

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 10, 2014

ModifyTypePreservingSTC now have the exact same bug as Unqual?

Good catch. I also fixed ModifyTypePreservingSTC.

@monarchdodra
Copy link
Collaborator

Auto-merge toggled on

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 10, 2014

Thanks!

else static if (is(T U == shared(inout const U))) alias Unqual = U;
else static if (is(T U == shared(inout U))) alias Unqual = U;
else static if (is(T U == shared( const U))) alias Unqual = U;
else static if (is(T U == shared( U))) alias Unqual = U;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style question: Would static if (is(T U == shared inout const U)) work, or are those parens necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of cause shared inout const U will work. Less parenthesis is better?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Less parenthesis is better?

I guess, it looked like a weird "hybrid" style to me, so I was wondering if there was a reason for it. If there is no reason, I'd think it's better to use shared inout const U

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, updated.

@braddr
Copy link
Member

braddr commented Feb 10, 2014

Pull updated, auto_merge toggled off

@monarchdodra
Copy link
Collaborator

Auto-merge toggled on

monarchdodra added a commit that referenced this pull request Feb 10, 2014
[REG2.065a] Issue 12089 - std.utf.validate and inout(char[]) failts to compile
@monarchdodra monarchdodra merged commit a04db41 into dlang:master Feb 10, 2014
@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 10, 2014

Thanks again!

@9rnsr 9rnsr deleted the fix12089 branch February 10, 2014 17:04
9rnsr pushed a commit to 9rnsr/phobos that referenced this pull request Feb 11, 2014
[REG2.065a] Issue 12089 - std.utf.validate and inout(char[]) failts to compile
@monarchdodra
Copy link
Collaborator

Good catch.

Hum... There's also the list of MutableOf, ConstOf, SharedInoutOf etc... that also need update. They are just package, but still, it is better to have code which is consistently correct.

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.

3 participants