On Mon, Jun 25, 2012 at 9:42 PM, Nico Weber <[email protected]> wrote:
> Hi Ted, > > On Mon, Jun 25, 2012 at 12:40 PM, Ted Kremenek <[email protected]> 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. > Would it be cheaper to just check whether the MemberExpr directly sets one member (e.g. var_ = var_)? I think cases like t->s_->a_ = t->s_->a_; are very rare. Is it enough to do measure the impact on a full build of chromium, or > would you like to see more data? > The question is whether code with this kind of bug will be commonly committed to a codebase. My guess would be that this would lead to bugs that are painstakingly debugged before committing. Thus, the warning might fire much more often during development. But I don't have data. Daniel > > 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 <[email protected]> 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 > >> [email protected] > >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
