2011/5/30 Eli Friedman <[email protected]> > On Mon, May 30, 2011 at 2:40 PM, Argyrios Kyrtzidis <[email protected]> > wrote: > > On May 30, 2011, at 2:36 PM, Argyrios Kyrtzidis wrote: > > > > Hi Nico, > > On May 30, 2011, at 12:19 PM, Nico Weber wrote: > > > > Hi Argyrios and Matthieu, > > this warning found a few problems in chromium – thanks! However, it also > > finds a few false positives, so I don't think I can turn it on by > default, > > which is a bummer. > > All of the false positives are of this form: > > class SomeInterface { > > public: > > virtual void interfaceMethod() {} // or = 0; > > protected: > > ~SomeInterface() {} > > } > > class WorkerClass : public SomeInterface { > > public: > > // many non-virtual functions, but also: > > virtual void interfaceMethod() override { /* do actual work */ } > > }; > > void f() { > > scoped_ptr<WorkerClass> c(new WorkerClass); // simplified example > > } > > This is a somewhat standard pattern (see > > e.g. http://www.gotw.ca/publications/mill18.htm, "Virtual Question #2"). > > Do you have any good suggestions how to deal with this case? > > > > If WorkerClass gets subclassed in the future, deletion in "f()" will be > > undefined behavior. Ideally WorkerClass should be marked "final" and then > > there will also be no warning; does this sound reasonable ? > > > > Um, to be exact, undefined behavior if you delete a WorkerClass * > pointer. > > Sure, except that there isn't any way to do that outside of C++0x mode. :) > > -Eli >
Hi Nico, first of all, thanks for field-testing the warning on the chromium code base, and I am glad it helped you spot some bugs :) I've tried to make the warning as tight as possible, to prune as much false-positives as possible however there is one difficulty: I only reason about the static type. I do not know if Clang has information about the dynamic type in the AST. In general it's a hard problem (and requires Inter-Procedural Analysis) and out of reach for a "simple" warning. It could perhaps be added to the Static Analyzer. If you can, I would suggest using the "final" attribute: class WorkerClass final: public SomeInterface (this could be activated only on the Clang build) If you cannot, unfortunately I cannot see anything (yet) to prune out this case. Perhaps that someone will have a genial idea ? -- Matthieu
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
