njames93 added a comment.

The default behaviour of this check should be transform void return types as 
that's how it has been since the check was first created. Adding an option 
which defaults to changing this behaviour would be harmful to current users of 
this check.



================
Comment at: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:132
+      RewriteVoidReturnType(
+          Options.getLocalOrGlobal("RewriteVoidReturnType", false)) {}
+
----------------
There's no reason this should be accessible globally.


================
Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize/use-trailing-return-type.rst:73-76
+.. option:: IgnoreVoidReturnType
+
+   When `true`, the check will not rewrite function signature for functions
+   with void return type. Default is `true`.
----------------
bernhardmgruber wrote:
> I picked up the habit to word conditions/options in a positive manner. So I 
> would rather name the option `RewriteVoidReturnType` instead of 
> `IgnoreVoidReturnType` and use the inverted Booleans throughout the code. The 
> reason is that my brain parses e.g. `if (!RewriteVoidReturnType)` easierly 
> than `if(!IgnoreVoidReturnType)` because of the double negation.
> 
> Feel free to debate me here. This is not a strong opinion.
Please change this back to `IgnoreVoidReturnType` or maybe even `IgnoreVoid`, A 
lot of checks use Ignore options to help restrict what the check will report 
on, so we should follow that convention.
If you want to avoid the double negatives that can be achieved by negative the 
value when you read and write the option from/to the options map.
```lang=c++
RewriteVoidReturnType(!Options.get("IgnoreVoid", false))
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135822/new/

https://reviews.llvm.org/D135822

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to