Skip to content
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

Issue 7516 - Postblit not called for structs returned from a ternary operator #4805

Merged
merged 1 commit into from Feb 5, 2016

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jul 7, 2015

https://issues.dlang.org/show_bug.cgi?id=7516

Today, front-end layer handles most of postblit calls, so it's easily fixable.

@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 30, 2015

Rebased on master, and ready for ddmd.

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
7516 Postblit not called for structs returned from a ternary operator

@ibuclaw
Copy link
Member

ibuclaw commented Feb 4, 2016

Looks OK. Though DoCopyOrMove doesn't roll off the tongue so well. :-)

Looks like there are failing tests too, though haven't checked why.

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 4, 2016

There was a small merge conflict. Fixed.

@@ -1564,10 +1585,11 @@ extern (C++) bool functionParameters(Loc loc, Scope* sc, TypeFunction tf, Type t
}
else
{
// arg = arg->isLvalue() ? callCpCtor(sc, arg) : valueNoDtor(arg);
//arg = doCopyOrMove(sc, arg);
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bad indent in comment had been introduced by ddmd conversion. Originally the the comment start // was at 0 column in C++ code.

Copy link
Member

Choose a reason for hiding this comment

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

To be fair, the comment should not have been starting at column zero, so it was already badly indented before ddmd.

Copy link
Member

Choose a reason for hiding this comment

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

I don't mean the indent. Why is there a piece of commented-out code without explanation in an otherwise-empty else branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can see from the git history, when Walter had added imrovements for throwing exceptions during function arguments evaluation, this line was commented out. So today, it's surely a dead code and IMO it won't be enabled again.

I've just removed the empty else branch.

@dnadlinger
Copy link
Member

Auto-merge toggled on

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 5, 2016

Thanks!

dnadlinger added a commit that referenced this pull request Feb 5, 2016
Issue 7516 - Postblit not called for structs returned from a ternary operator
@dnadlinger dnadlinger merged commit 1ea6991 into dlang:master Feb 5, 2016
@9rnsr 9rnsr deleted the fix7516 branch February 7, 2016 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants