I hope we can detect more unnecessary conversions/copies

Clean Code New rules / language support
1

For reference, we already have C++ static code analysis to find redundant casts. However, it appears this will only find c-style casts and named casts. This leaves functional casts and braced init as undetected.

I’m looking at a context where having better rules would help discover cases where previously useful conversions are now redundant.

Consider the existance of a strong type in the interface, to separate from int. For the sake of the argument, consider something like

struct Id
{
    int id = 0;
};

If we gradually adopt this type in our code base, then it’s plausible that we would have many instances
of function calls similar to

someFunction(int{someId});
someOtherFunction(Id{someInt});

When driving adoption further through refactoring, one of these conversions is easier to detect than the other. Presumably, someInt will have it’s type changed from int to Id, and plausibly renamed to someId, however, this is due to other parts of the surrounding code. Meanwhile, the conversion around someId will cause a compilation failure, and the conversion will simply be removed.

Either way, it would be useful to have better tooling to clean up leftover unnecessary conversions.

This might fall under a notion of an unnecessary copy, when you pass the argument into a function that takes the parameter by value (or potentially const ref).

I’m not sure if there are any plans to make a rule that suggests modernization into C++23’s auto(x)/auto{x}. Although I suspect that such a rule wouldn’t necessarily be meant to catch cases where the copy is unintended, this might be a convenient motivation to point at code snippets that deserve attention.

5

Hi @torgeir.skogen, and thank you for your suggestions!

I think we have other rules similar to what you have in mind, but only for specific situations: S6231 and S6011.

I can see some interest in what you suggest, but only if the copied value is directly passed as an argument to a function that takes it by value. In other situations, maybe you actually wanted to create a copy (in the case of a const & param, copying can avoid aliasing). So it’s going to be quite focused, but the use case of strong types may be prevalent enough to make it useful. I’m creating a ticket to track this idea.

About auto{x}, while we are currently looking at C++23, we have not yet considered a rule that would suggest replacing T[t} with auto {t}. I think my feeling is that the benefit is not always obvious, unless T is hard to spell out. I find the syntax based on auto quite obscure. Maybe I just need to get used to it.

I’m going to think more about it with my colleagues.

2 Likes
6

Hi @torgeir.skogen,

Small update for to rule pushing toward using auto {x}. For now, we decided not to move forward with such a rule. We think the cases where the benefit would be non-controversial were probably not common enough to justify a dedicated rule.

7

I understand. I mostly brought it up as a potential coincidence factor more than thinking about it in terms of its own merits.

8

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.