Improve std.container.util.make with forwarding and clearer errors#10973
Improve std.container.util.make with forwarding and clearer errors#10973shhreya13 wants to merge 1 commit intodlang:masterfrom
Conversation
|
Thanks for your pull request and interest in making D better, @shhreya13! 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. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + phobos#10973" |
There was a problem hiding this comment.
This PR has clearly had very little effort put into its preparation. Judging from the formatting of the PR message and the content of the submitter's Github account, I suspect that it was generated by an LLM with minimal human oversight.
Given the above, I think it is unlikely that this PR will ever result in any useful contribution to Phobos, but I will leave it open for now to give its human submitter the chance to fix these shortcomings.
| This module contains common utilities used by containers. | ||
|
|
||
| This module is a submodule of $(MREF std, container). | ||
|
|
||
| Source: $(PHOBOSSRC std/container/util.d) | ||
|
|
||
| Copyright: 2010- Andrei Alexandrescu. All rights reserved by the respective holders. | ||
| Copyright: 2010- Andrei Alexandrescu. | ||
|
|
||
| License: Distributed under the Boost Software License, Version 1.0. | ||
| (See accompanying file LICENSE_1_0.txt or copy at $(HTTP | ||
| boost.org/LICENSE_1_0.txt)). | ||
|
|
||
| Authors: $(HTTP erdani.com, Andrei Alexandrescu) | ||
|
|
||
| $(SCRIPT inhibitQuickIndex = 1;) | ||
| Authors: Andrei Alexandrescu |
There was a problem hiding this comment.
Please do not include unrelated documentation changes in this PR.
| Creates and returns an initialized instance of type `T`. | ||
|
|
||
| This utility abstracts the difference between constructing | ||
| structs and classes. | ||
|
|
||
| Structs are constructed using: | ||
| --- | ||
| T(args) | ||
| --- | ||
|
|
||
| Classes are constructed using: | ||
| --- | ||
| new T(args) | ||
| --- | ||
|
|
||
| This allows generic container code to construct objects | ||
| without worrying about whether `T` is a struct or class. | ||
| */ |
There was a problem hiding this comment.
Please do not include unrelated documentation changes in this PR.
| { | ||
| T make(Args...)(Args arguments) | ||
| if (is(T == struct) && __traits(compiles, T(arguments))) | ||
| @safe T make(Args...)(Args arguments) |
There was a problem hiding this comment.
make is a template function, which means that attributes like @safe are inferred automatically by the compiler. It should not be necessary to add an explicit @safe attribute here.
If there is something in the function body that prevents @safe from being inferred when it ought to be, the body should be changed to make that inference possible.
|
|
||
| T make(Args...)(Args arguments) | ||
| if (is(T == class) && __traits(compiles, new T(arguments))) | ||
| @safe T make(Args...)(Args arguments) |
There was a problem hiding this comment.
See above re: inference of @safe.
| * Convenience function for constructing a generic container. | ||
| */ | ||
| Convenience function for constructing a generic container. | ||
| */ |
There was a problem hiding this comment.
Please do not include unrelated formatting changes in this PR.
| $(SCRIPT inhibitQuickIndex = 1;) | ||
| Authors: Andrei Alexandrescu | ||
| */ | ||
|
|
There was a problem hiding this comment.
Please do not include unrelated formatting changes in this PR.
|
|
||
| auto arr1 = make!(Array!dchar)(); | ||
| assert(arr1.empty); | ||
|
|
There was a problem hiding this comment.
Please do not include unrelated formatting changes in this PR.
|
|
||
| auto rtb1 = make!(RedBlackTree!dchar)(); | ||
| assert(rtb1.empty); | ||
|
|
There was a problem hiding this comment.
Please do not include unrelated formatting changes in this PR.
| auto d = make!(DList!int)(1,2,3,4,5); | ||
| assert(a == b); // this better terminate! | ||
|
|
||
| assert(a == b); |
There was a problem hiding this comment.
Please do not include unrelated formatting changes in this PR.
|
|
||
| module std.container.util; | ||
|
|
||
| import std.algorithm.mutation : forward; |
There was a problem hiding this comment.
| import std.algorithm.mutation : forward; | |
| import core.lifetime : forward; |
The fact that this import is incorrect suggests that you did not even attempt to compile or test this change on your own device before submitting a PR. Please do so in the future; it will save both you and us a lot of time.
0xEAB
added
the
Review:AI Generated
4 participants
Summary
This PR improves the
maketemplate instd.container.util.Changes
std.algorithm.mutation.forwardwhen passing constructor arguments to preserve move semantics and avoid unnecessary copies.static assertwhen a constructor cannot be called with the provided arguments.makeabstracts the difference between struct construction (T(args)) and class construction (new T(args)).Motivation
These changes improve developer experience by providing clearer error messages and slightly improving argument handling while keeping existing behavior unchanged.
Related Issue
Closes #10972