llvm/utils/TableGen/Common/CodeGenRegisters.h:571: clumsy parameter passing ?

[dcb314]

Static analyser cppcheck says:

llvm/utils/TableGen/Common/CodeGenRegisters.h:571:26: performance: Function parameter 'Name' should be passed by const reference. [passedByValue]

Source code is

RegUnitSet(std::string Name) : Name(Name) {}

[topperc]

Would adding std::move be better? At least one caller passes a temporary string

RegUnitSet(std::string Name) : Name(std::move(Name)) {}

[dcb314]

Would adding std::move be better?

No idea. The tool merely suggests changing the function prototype:

RegUnitSet( const std::string & Name)

Code in the initialiser list isn't covered by this bug report.

[RKSimon]

I'd love it if we could agree in the coding standard when to use pass by ref, move and && constructor/assignment - the closest guideline we have is to avoid copies when using auto in iterators.

[dcb314]

In my experience in the last 35+ years with C++, almost all coding standards
that rely on the programmer to maintain standards, are ignored.

To be useful, standards need to be enforced, probably by the compiler
or a static analyser.

I note 132 examples of the passedByValue issue in the following list of
directories in the llvm project:

trunk/clang/
trunk/llvm/
trunk/flang
trunk/lib*

I ignore other directories in the llvm project.

[firewave]

Would adding std::move be better? At least one caller passes a temporary string

I communicated that before that the suggestion change might not be the proper solution.

There is an upstream ticket about adjusting the message: https://trac.cppcheck.net/ticket/12384.

[topperc]

Would adding std::move be better?

No idea. The tool merely suggests changing the function prototype:

RegUnitSet( const std::string & Name)

Code in the initialiser list isn't covered by this bug report.

If you change the code to use std::move as I suggested, does the tool stop suggesting to make a change?

[firewave]

If you change the code to use std::move as I suggested, does the tool stop suggesting to make a change?

Yes.

[dcb314]

If you change the code to use std::move as I suggested, does the tool stop suggesting to make a change?

Yes.

I agree that cppcheck will shut up if std::move is used, but is using std::move a better solution
than using constant reference ? If so, how ?

I've done some reading in the area and despite multiple attempts, I am still clueless
as to why std::move is better than constant reference.

I guess that if std::move is moving something from one place to another then a memmove()
will be called and that can be expensive for some types.

I am somewhat reluctant to use a feature I can't understand and for which I
can't see a definite benefit. KISS principle applies here, I think.