[cfe-commits] [patch] Warn on self assignment for member variables

[Nico Weber]

Hi Ted,

On Mon, Jun 25, 2012 at 12:40 PM, Ted Kremenek <kremenek at apple.com> wrote:
> Hi Nico,
>
> How expensive is this check?  This code makes me nervous:

I didn't measure, but note that this is only executed if the lhs and
rhs are both MemberExprs, and they both refer to the same Decl. This
isn't true for most assignments.

Is it enough to do measure the impact on a full build of chromium, or
would you like to see more data?

Nico

>
> +  llvm::FoldingSetNodeID lhsID, rhsID;
> +  ML->getBase()->Profile(lhsID, Sema.Context, true);
> +  MR->getBase()->Profile(rhsID, Sema.Context, true);
> +  if (lhsID == rhsID)
> +    Sema.Diag(Loc, diag::warn_identity_memvar_assign);
>
> If this is done on every assignment, might this be a bit expensive for marginal gain?
>
> On Jun 25, 2012, at 12:26 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> Hi,
>>
>> the attached patch adds a warning for self-assignments of member
>> variables. This is PR13104. I do this every now and then locally when
>> writing setter functions: I type "void set_var(int var) { var_ =" and
>> then try to hit ctrl-p twice to complete the lhs to "var" in vim, but
>> miss and hit it only once, so that I end up with "var_ = var_;"
>>
>> This finds 0 bugs and 0 false positives in chromium, so I'm not sure
>> how useful this is. Opinions?
>>
>> Nico
>> <clang-memvar-assign.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>