Fix #9811 Remove const from Tuple.toString#10646
Merged
thewilsonator merged 1 commit intodlang:masterfrom Feb 22, 2025
Merged
Conversation
Now, member structs won't become const due to transitiveness and the code in `std.format` can choose (erroneously) non-cost toString() functions of member structs. An expert has to decide whether this is worth the breaking change. Now, code that uses `const` tuples and calls toString directly on them will break. Alternatively, we could use [Template this parameters](https://dlang.org/spec/template.html#template_this_parameter). That way, we get a template parameter with the real type of this. Then we can cast away constness of `this` when we now for certain that it isn't, since `is(T != typeof(this))` (where T is the template this parameter). Of course, this implies making toString `@trusted` too. This might also lead to unforeseen bugs when `const` is cast away but the member objects are actually const. I'm not sure how this works. Fwiw, currently std.format and `std.conv: to` already intercepts const tuples, thus it at least won't call toString directly. The breaking change will only effect code that calls toString on const tuples directly. That's why I have added non-prescritive tests. If something in std.format changes, they'll be alerted and can then decide whether to change the tests or whether this module also needs work, in case this would lead a bigger breaking change. Followup to dlang#10645.
Contributor
|
Thanks for your pull request and interest in making D better, @Inkrementator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
|
thewilsonator
approved these changes
Feb 22, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now, member structs won't become const due to transitiveness and the code in
std.formatcan choose (erroneously) non-cost toString() functions of member structs.An expert has to decide whether this is worth the breaking change. Now, code that uses
consttuples and calls toString directly on them will break. Alternatively, we could useTemplate this parameters.
That way, we get a template parameter with the real type of this. Then we can cast away constness of
thiswhen we now for certain that it isn't, sinceis(T != typeof(this))(where T is the template this parameter). Of course, this implies making toString@trustedtoo.This might also lead to unforeseen bugs when
constis cast away but the member objects are actually const. I'm not sure how this works.Fwiw, currently std.format and
std.conv: toalready intercepts const tuples, thus it at least won't call toString directly. The breaking change will only effect code that calls toString on const tuples directly.That's why I have added non-prescritive tests. If something in std.format changes, they'll be alerted and can then decide whether to change the tests or whether this module also needs work, in case this would lead a bigger breaking change.
Followup to #10645.