[SERVER-81147] Complain about uninitialized members in struct Created: 18/Sep/23 Updated: 19/Sep/23 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Vishnu Kaushik | Assignee: | Backlog - Service Architecture |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Assigned Teams: |
Service Arch
|
||||||||
| Participants: | |||||||||
| Description |
|
Recently I had a patch where I converted a std::pair<int, vector> to a struct to improve readability:
This change introduced the bug that while std::pair initializes the values to defaults (so the int gets initialized to 0), the struct doesn't do this, meaning that there might be a garbage value in the errCode field. Can we add a clang-tidy rule to cover this case? Or does Coverity already cover it? See the comments for more info; it seems a memory sanitizer may or may not catch this bug. |
| Comments |
| Comment by Billy Donahue [ 18/Sep/23 ] |
|
(I'll recap what I added to the above-quoted thread) " I think ideally a clang-tidy rule would complain that I'm creating a struct that doesn't specify initial values for its primitive members." I don't think we would go for that. The ability to leave data members uninitialized is an optimization. You can make a trivial aggregate that way and never pay to initialize it. I would not want a static check for this to break a build of correct and efficient code. I think it's more of a user-side problem than a class-side problem. When declaring an instance of that pair-like type, if you need it to be value-initialized you would need to use curly-braces. So the declaration expresses the intent to initialize it. Other users might not need to initialize it. We can't rely on types having default values generally. It feels like one of those things we just need to get used to in C++. If we get FYI annotations in VSCode, this would be a good one. |