[SERVER-77897] Enable the performance-no-automatic-move clang-tidy check Created: 07/Jun/23  Updated: 27/Jun/23  Resolved: 16/Jun/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: New Feature Priority: Major - P3
Reporter: Alex Neben Assignee: Steve Gross
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Participants:

 Description   

Enable the following clang-tidy check https://releases.llvm.org/12.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-no-automatic-move.html

 

 



 Comments   
Comment by Alex Neben [ 16/Jun/23 ]

Absolutely! Congrats on closing your first ticket.

Comment by Steve Gross [ 16/Jun/23 ]

Committed https://github.com/10gen/mongo/pull/13643; juan.gu@mongodb.com, should I close this ticket as complete now?

Comment by Steve Gross [ 15/Jun/23 ]

Got CI feedback; fixed up a few more little things and re-triggered the PB

Comment by Steve Gross [ 15/Jun/23 ]

Fixed up the references; now I'll try to get a CI build to run to confirm the new clang-tidy warnings are addressed.

Comment by Steve Gross [ 14/Jun/23 ]

Per offline thread w/ juan.gu@mongodb.com: I clarified that I'll fix up the 63 broken references, and put it in a single PR for review by this team.

Comment by Juan Gu [ 14/Jun/23 ]

you can run a full PB to ensure your changes wouldn't break anything. When you submit your PB, you can schedule all the tasks under all the build variants which start with '!', '!' means required here. 

Comment by Steve Gross [ 14/Jun/23 ]

Given that the failures cover a wide swath of code, does that mean I'll need lots of reviewers (from each area of code affected)? If so, it seems easier to break it up into a few smaller PRs...?

Comment by Juan Gu [ 14/Jun/23 ]

Yes, we normally fix all the failures first, then run a patch build from Evergreen with the task 'clang_tidy' and 'commit queue'. After confirming it works, we can push it for review. Once approved, it can then be merged into the master branch

Comment by Steve Gross [ 14/Jun/23 ]

In this branch, I have added the new check and fixed up a single .cpp file to show how to address the new clang-tidy failures. Given that there are 63 failures, what's the normal procedure here? Do we fix up all the failures first before merging in the new check?

cc: juan.gu@mongodb.com

Comment by Steve Gross [ 14/Jun/23 ]

There are 63 failures due to the newly added clang-tidy check. I'm reading up more on the check itself to better understand what it takes to fix an instance.

Comment by Steve Gross [ 14/Jun/23 ]

I added the check, ran it in Evergreen, and confirmed it failed (log). Next step is to comb through the results to understand if/how it's attributable to the newly added check.

Generated at Thu Feb 08 06:36:54 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.