On Wed, Feb 17, 2016 at 2:39 AM, Daniel Marjamäki
<daniel.marjam...@evidente.se> wrote:
> danielmarjamaki marked 2 inline comments as done.
>
> ================
> Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:15
> @@ +14,3 @@
> +
> + // warning here; p should be const
> + char f1(char *p) {
> ----------------
> LegalizeAdulthood wrote:
>> With pointers, there are always two layers of constness:
>>
>>
>> - Whether the pointer itself is modified
>> - Whether the data pointed to is modified
>>
>> When you say "p should be const", I read that as the former.
>>
>> I am pretty sure you intended the latter. You should be more explicit in
>> your documentation. The ambiguity would have been resolved if you showed
>> the "after" version of the code that would eliminate the warning. This is
>> semi-implied by your next example, but it's kinder to the reader to resolve
>> this ambiguity immediately.
>>
>>
> ok I understand, will try to improve
>
> ================
> Comment at: test/clang-tidy/readability-non-const-parameter.cpp:3
> @@ +2,3 @@
> +
> +// Currently the checker only warns about pointer arguments.
> +//
> ----------------
> LegalizeAdulthood wrote:
>> How hard is it to extend it to references?
>>
>> Certainly the confusion about what is const is easier to resolve in the case
>> of references because the references themselves are immutable.
> If a "int &" reference parameter is not written then probably it's better to
> pass it by value than making it const. I would prefer that unless it has to
> use "int &" to comply with some interface.
I think it makes sense to pass it by value only if it is not an
expensive-to-copy type (we have an AST matcher helper for that in
TypeTraits.h), or to comply with an interface.
~Aaron
>
> I realize that the same can be said about pointers. If there is a "int *p"
> and you just read the value that p points at .. maybe sometimes it would be
> preferable to pass it by value.
>
>
> http://reviews.llvm.org/D15332
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
