Skip to content

Conversation

@WalterBright
Copy link
Member

This implements implicit conversions of (ptr+offset) expressions so that the ptr part determines if the expression is implicitly convertible.

This is a reboot of #1700

http://wiki.dlang.org/DIP29

@WalterBright
Copy link
Member Author

Just ignore the testCols.d thing, it's just that damned CRLF problem again that git thinks is different but is not.

Copy link
Contributor

Choose a reason for hiding this comment

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

This line will make an invalid pointer. I think we should avoid any undefined behaviors even in trivial test cases, if it's not intended.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@9rnsr
Copy link
Contributor

9rnsr commented Feb 22, 2014

Note that, CRLF fix for testCols.d is already done #3307 in git head.so just you can remove the commit.

src/cast.c Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be static?

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, change this to return void and use t and result member variables.

Copy link
Member Author

Choose a reason for hiding this comment

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

made it static

@WalterBright
Copy link
Member Author

Note that, CRLF fix for testCols.d is already done #3307 in git head.so just you can remove the commit.

Great, but I have no idea how to do that.

@yebblies
Copy link
Contributor

Great, but I have no idea how to do that.

git checkout master
git pull --ff-only upstream master
git checkout DIP29_1
git rebase -i master
<delete the commit you don't want from the list>
git push -f

@WalterBright
Copy link
Member Author

@yebblies that worked like a champ! You da man!

Copy link
Contributor

Choose a reason for hiding this comment

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

Why tail immutable instead of full immutable? The immutableness of the head shouldn't make any difference here...

Copy link
Member Author

Choose a reason for hiding this comment

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

Either way works, but I wished to make it clear that what matters with pointer conversions is what is pointed to, not the pointer itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is clearer. Put that note in a comment instead of complicating the code (and making it slower).

@WalterBright
Copy link
Member Author

I should add that this should work like a pure function:

pure T* foo(S* p) { return p + offset; }

@9rnsr
Copy link
Contributor

9rnsr commented Feb 23, 2014

pure T* foo(S* p) { return p + offset; }

Only when S* is implicitly convertible to T*? Without concrete full code, I cannot see it is correct or not.

@yebblies
Copy link
Contributor

Should have a bugzilla report, not just a DIP.

@WalterBright
Copy link
Member Author

Sorry, should be:

pure T* foo(S* p) { return cast(T*)(p + offset); }

@yebblies , the last time everyone said it should be a dip.

@yebblies
Copy link
Contributor

the last time everyone said it should be a dip.

Having a DIP is great, but you still need a bugzilla entry for the changelog.

@WalterBright
Copy link
Member Author

@9rnsr
Copy link
Contributor

9rnsr commented Feb 25, 2014

Looks good.

@9rnsr
Copy link
Contributor

9rnsr commented Feb 25, 2014

Auto-merge toggled on

9rnsr added a commit that referenced this pull request Feb 25, 2014
DIP29: implicit conversion of pointer expressions
@9rnsr 9rnsr merged commit 64b8082 into dlang:master Feb 25, 2014
@WalterBright WalterBright deleted the DIP29_1 branch February 25, 2014 18:07
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