Page MenuHomePhabricator

[clangd] Trivial setter support when moving items to fields

Authored by njames93 on Sep 25 2020, 6:01 AM.



Extend the Trivial setter documentation to support cases where the value is moved into a field using std::move.

Diff Detail

Event Timeline

njames93 created this revision.Sep 25 2020, 6:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2020, 6:01 AM
njames93 requested review of this revision.Sep 25 2020, 6:01 AM
sammccall accepted this revision.Sep 25 2020, 6:41 AM

Nice, thanks!


nit: you could skip this check if you like, the other variant isn't going to be on the RHS of an assignment :-)


I guess you want !ND->getIdentifier() || ND->getName() != "move" || ... to guard against it being a special name somehow - getName() asserts in that case.

This revision is now accepted and ready to land.Sep 25 2020, 6:41 AM
kadircet added inline comments.Sep 25 2020, 6:42 AM

can you also update the docs?


nit: combine with the next condition, and perform the string comparison last, i.e.:

if(!ND || !ND->isInStd || ND->getName() != "move")

njames93 updated this revision to Diff 294357.Sep 25 2020, 9:57 AM
njames93 marked 2 inline comments as done.

Updated function doc
Fix potential assertion


You never know what crazy code people could concoct. It could be argued that this has a performance win by easily filtering out some bad candidates quickly and avoid needing to run the (slightly) more expensive checks on the name later down the line :-)


Can you explain the reasoning for moving the comparison to the end?

kadircet accepted this revision.Tue, Sep 29, 12:28 PM
kadircet added inline comments.

it was to skip a potentialyl expensive string comparison, but now that I look into isInStdNamespace it is also performing a string comparison in the end. so nvm.

This revision was landed with ongoing or failed builds.Tue, Sep 29, 1:51 PM
This revision was automatically updated to reflect the committed changes.