Implement VEC_NEW internal lint #62678
Closed
Conversation
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
rust-highfive
added
the
S-waiting-on-review
|
Seriously? EDIT: I'm just always using |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I guess for consistency? But as stated above: This was easy to implement and not really much work to fix the fallout. We can throw this on the pile of internal lints, that didn't make it, no harm done. :) |
|
I'd be interested in knowing whether this has an impact on compilation times, as it leads to macro expansion occurring -- which internally will still call Maybe it makes sense to do the opposite or nothing here? |
|
The macro expands to Lines 145 to 152 in 85a360e
But if this is the case, wouldn't it make sense to add a special case for |
|
Note, I don't mean the runtime cost -- I expect that to be equivalent -- I mean purely the macro parsing/expansion overheads. I don't think these are significant; but, well, if we're going to ban |
Oh yeah of course. It's late here Is there some way I can benchmark the compile times? |
|
I actually looked for a similar lint in the Clippy and couldn't find it (to prefer |
|
☔ The latest upstream changes (presumably #62670) made this pull request unmergeable. Please resolve the merge conflicts. |
|
cc @eddyb |
|
I prefer this but we should have some discussion and find some consensus. @petrochenkov I would also prefer to avoid macros if we had a way to make As it stands, |
|
@eddyb |
|
Why does the compiler want it this way? There's no justification in #49509 (comment) |
|
Ping from Triage. Is any more review forthcoming or should this be back in the author's hands? @flip1995 @oli-obk @Mark-Simulacrum @eddyb |
|
I don't think we find a consensus here. I think this should be discussed in #49509 first and then picked up again. |
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.
And another one. That was an easy one to implement, but the fallout was pretty big. Thanks to regex (
s/([\(\s])Vec::new()/\1vec![]/g) this was quite pleasant to fix :)cc #49509 (comment)
r? @oli-obk