clang-tidy: check return style

[Quuxplusone]

Bugzilla Link	PR22416
Status	NEW
Importance	P enhancement
Reported by	Eugene Zelenko (eugene.zelenko@gmail.com)
Reported on	2015-01-30 16:08:10 -0800
Last modified on	2016-01-18 11:01:39 -0800
Version	unspecified
Hardware	All All
CC	adrian@adek.io, alexfh@google.com, djasper@google.com, klimek@google.com, legalize@xmission.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Hi!

I think will be good idea to have checks for style of returns in non-void functions as part of readability checkers.

Round brackets should be used only for expressions, not around variable/member.

Eugene.

[Quuxplusone]

Please provide a couple of examples of what specific patterns you want to address with an explanation of why this matters. If there are any style guides that have this rule, please add a pointer to them.

[Quuxplusone]

Hi, Alexander!

Even such trivial situations like:

return (true);

return (d_ClassMember);

Ideally it's great it will be replaced with:

return true;

return d_ClassMember;

Eugene.

[Quuxplusone]

It looks like a matter of readability only. Implementing this check is a good task for someone who wants to start contributing to clang-tidy ;)

[Quuxplusone]

(In reply to comment #1)
> Please provide a couple of examples of what specific patterns you want to
> address with an explanation of why this matters. If there are any style
> guides that have this rule, please add a pointer to them.

Google Style guide: http://google-
styleguide.googlecode.com/svn/trunk/cppguide.html#Return_Values

[Quuxplusone]

Direct link to specific section in google style guide:

https://google.github.io/styleguide/cppguide.html#Return_Values

[Quuxplusone]

FYI, since it came up in my review of redundant return statements, I started making a check for this. I think I just need to check if dyn_cast(Return->getRetValue()) is non-zero and then it means it is of the form:

return(expr);

and not

return expr;

However, I haven't verified this yet.

[Quuxplusone]

Ah, I hav a diff for that: http://reviews.llvm.org/D16286
I used a similar matcher there.