Fix compilation with DIP1000 and compatibility with future in=scope const dmd change
#1791
Conversation
When the `in` parameter storage class changes its meaning to `scope const` we would get the following errors: source/dub/dependency.d(83): Error: scope variable `ver` assigned to `this` with longer lifetime source/dub/dependency.d(84): Error: scope variable `ver` assigned to `this` with longer lifetime This commit fixes this.
|
Thanks for your pull request, @PetarKirov! |
PetarKirov
changed the title
in=scope const dmd changein=scope const dmd change
PetarKirov
force-pushed
the
fix-in-param-attr
branch
from
bc6d632 to
c04f9ca
Compare
PetarKirov
force-pushed
the
fix-in-param-attr
branch
from
c04f9ca to
56307b3
Compare
| if (platform.platform.canFind("windows")) | ||
| return settings.targetName ~ ".exe"; | ||
| else return settings.targetName; | ||
| else return settings.targetName.idup; |
There was a problem hiding this comment.
Couldn't this copy be avoided by annotating settings with return?
There was a problem hiding this comment.
Yes, that should work and I thought about that, but I decided not to change the interface. I think it's more of compiler deficiency that it doesn't recognize that settings.targetName in GC allocated and so it has infinite lifetime, and so it's safe to escape it.
There was a problem hiding this comment.
Actually I now remember why I didn't add the return attribute. The reason is that in all, but this single case, this function doesn't escape any part of it's parameter. Although this may sound too theoretical, in principle adding return attribute essentially limits callers of the function, meaning that just because of this single case they will need ensure that lifetime of settings is as long as the one of the result, which is mildly annoying.
A better solution than the idup is a function (with potentially the same name) that returns the argument as it is (without copying) if it is immutable with infinite lifetime.
|
One of the AppVeyor builds is failing with out of memory since several other PRs, so it's unrelated. |
|
Thanks! |
4 participants
No description provided.