Fix QStringBuilder taking reference to temporary due to auto
ClosedPublic

Authored by kossebau on Jul 19 2018, 9:53 PM.

Details

Summary

Compiler decides for "auto" type result to use QStringBuilder, though the
expression passed to it includes a temporary QString result from a
QStringList::join() result. Which then is dropped after the assignement, due
to getting out of scope.

BUG: 394055

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Jul 19 2018, 9:53 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptJul 19 2018, 9:53 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
kossebau requested review of this revision.Jul 19 2018, 9:53 PM
kossebau added inline comments.Jul 19 2018, 9:54 PM
plugins/clang/codecompletion/completionhelper.cpp
81

Any proposal for a comment to prevent someone "fixing" this into "auto" again?

kfunk accepted this revision.Jul 20 2018, 9:46 AM
kfunk added a subscriber: kfunk.

This does fix https://bugs.kde.org/show_bug.cgi?id=394055, doesn't it? If so please reference it in the commit message.

plugins/clang/codecompletion/completionhelper.cpp
81

Not sure if needed, one needs to pay attention to that when using auto combined with concatenating strings after all. At least clang-tidy will not /automatically/ transform this using auto.

This revision is now accepted and ready to land.Jul 20 2018, 9:46 AM

This does fix https://bugs.kde.org/show_bug.cgi?id=394055, doesn't it? If so please reference it in the commit message.

No idea, will give it a check and if so, add the reference, okay. Thanks for review.

plugins/clang/codecompletion/completionhelper.cpp
81

Many people might not be aware of that trap with QStringBuilder and auto. Or know what QStringBuilder is at all, as it hides away usually.
One has to be bitten a few times before it's part of mental background code checking perhaps :)

And I was more thinking of "human" fixers who are in for code consistency or modern looking code (/me tries to look innocent) ;)

Okay, so being unsure I will leave that comment away for now, too much generalizing from ego perhaps :)

kossebau edited the summary of this revision. (Show Details)Jul 20 2018, 10:38 AM
This revision was automatically updated to reflect the committed changes.
vkrause added inline comments.
plugins/clang/codecompletion/completionhelper.cpp
81

Another possibility might be to keep the auto and force the QString conversion on the right hand side. Slightly more verbose, but protects you from breaking this again.