On Nov 17, 2010, at 3:11 PM, Argyrios Kyrtzidis wrote:
> Author: akirtzidis
> Date: Wed Nov 17 17:11:54 2010
> New Revision: 119583
>
> URL: http://llvm.org/viewvc/llvm-project?rev=119583&view=rev
> Log:
> Introduce option -Wargument-larger-than[=N] which warns about function
> definitions if they take by-value
> or return by-value any POD that is larger than some threshold (default is 64
> bytes).
Very nice, some minor suggestions:
> def CharSubscript : DiagGroup<"char-subscripts">;
> +def ArgumentSizeLargerThan : DiagGroup<"argument-larger-than">;
Since this applies to return values also, how about making this be
-Wlarge-by-value-copy ?
> +def warn_parameter_size: Warning<"size of %0 is %1 bytes">,
> + InGroup<ArgumentSizeLargerThan>;
> +def warn_return_value_size: Warning<"return value of %0 is %1 bytes">,
> + InGroup<ArgumentSizeLargerThan>;
instead of just stating that it is N bytes, how about wording it like:
%0 is a large (%1 byte) pass-by-value argument; pass it by reference instead?
That makes it more clear what the suggested alternative is and why it's bad.
>
> +void Sema::DiagnoseSizeOfParametersAndReturnValue(ParmVarDecl * const *Param,
Please add a doxygen comment explaining what this is doing and a couple
comments in the code.
> + if (ReturnTy->isPODType() &&
> + Diags.getDiagnosticLevel(diag::warn_return_value_size) !=
> + Diagnostic::Ignored) {
Is this worth doing a 'is disabled' check for it? It doesn't seem that
expensive. Does it cause a lot of PCH deserialization or something?
-Chris
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
