This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Don't warn about call to unresolved operator*
ClosedPublic

Authored by malcolm.parsons on Feb 1 2017, 6:23 AM.

Details

Summary

The misc-unconventional-assign-operator check had a false positive
warning when the 'operator*' in 'return *this' was unresolved.

Change matcher to allow calls to unresolved operator.

Fixes PR31531.

Event Timeline

malcolm.parsons created this revision.Feb 1 2017, 6:23 AM
idlecode added inline comments.
clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
63

Seems that it will catch all unary operators with this as first argument.
e.g. in case operator- is defined somewhere, check will not warn about returning -this.
Do you think adding hasOverloadedOperatorName("*") for such abstract case is worth it?

clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
63

I'm just trying to suppress them warning from the template. The operator can be checked properly in an instantiation.

idlecode added inline comments.Feb 4 2017, 12:37 AM
clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
63

LGTM then :)

aaron.ballman accepted this revision.Feb 5 2017, 8:42 AM

LGTM with a minor nit that can be fixed without further review.

test/clang-tidy/misc-unconventional-assign-operator.cpp
92

You should put a comment near here to explain why this is a necessary part of the test.

Also, when fixing PRs, it's sometimes helpful to put the entire test into a namespace named after the PR (so people can do historical tracking a bit more easily). e.g., namespace pr31531. We've not done this often for clang-tidy, but it's quite common in clang's tests and might be a nice habit to get into -- it helps reduce accidental name collisions as a benefit.

This revision is now accepted and ready to land.Feb 5 2017, 8:42 AM
This revision was automatically updated to reflect the committed changes.
malcolm.parsons marked 4 inline comments as done.