tbourvon marked 4 inline comments as done.
tbourvon added a comment.
(By the way, credits go to @Abpostelnicu for the latest changes regarding
MaximumLineLength interop with clang-format options)
================
Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp:376
+ // expression wouldn't really benefit readability. Therefore we abort.
+ if (NewReturnLength > MaximumLineLength) {
+ return;
----------------
Abpostelnicu wrote:
> lebedev.ri wrote:
> > Is there really no way to format the fixes, and *then* check the line
> > length?
> > ```
> > $ clang-tidy --help
> > ...
> > -format-style=<string> -
> > Style for formatting code around applied
> > fixes:
> > - 'none' (default) turns off formatting
> > - 'file' (literally 'file', not a
> > placeholder)
> > uses .clang-format file in the closest
> > parent
> > directory
> > - '{ <json> }' specifies options inline,
> > e.g.
> > -format-style='{BasedOnStyle: llvm,
> > IndentWidth: 8}'
> > - 'llvm', 'google', 'webkit', 'mozilla'
> > See clang-format documentation for the
> > up-to-date
> > information about formatting styles and
> > options.
> > This option overrides the 'FormatStyle`
> > option in
> > .clang-tidy file, if any.
> > ...
> > ```
> > so `clang-tidy` is at least aware of `clang-format`.
> I think this is doable since I see this in the code:
>
> https://code.woboq.org/llvm/clang-tools-extra/clang-tidy/ClangTidy.cpp.html#199
>
> That leads me to think that we can have this before applying the fixes and in
> case the fix after re-format has a line that violates our rule it gets
> dropped. I'm gonna update the patch with this new addon.
Regarding this comment, Andi (@Abpostelnicu) and I have analyzed the situation
and we believe that formatting the replacement before checking the line length
achieves almost nothing, yet complicates the code a lot.
If we format the replacement before applying the fix, then we're //almost//
sure that clang-format will split the replacement into multiple lines and that
it will never go above the maximum line length (except for extremely rare cases
like 80+ characters identifiers).
Actually, we believe that splitting the return statement into multiple lines
hinders its readability, and therefore that in cases where the fix would exceed
the maximum line length, we're better off not applying it, since the main goal
of this check **is** readability.
clang-format can still be run after the fix is applying, of course.
https://reviews.llvm.org/D37014
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
