At 11:57 AM 11/2/2010, Ted Kremenek wrote: >On Nov 2, 2010, at 7:01 AM, Douglas Gregor wrote: > > > Sure, but why special-case simple function names? I'd bet that > checkers looking at function names don't check for the context of > the function (translation unit vs. namespace vs. class) or, if it > was an overloaded function, that we have the correct signature. > Shall we limit those, too? > > > > Why is a checker that looks at, say, attributes on callee > functions more complicated than one that looks at names? The AST is > simple in both cases, but you are suggesting that the former > require the advanced interface while the latter uses the simpler > one... even though, based on the issues I mentioned above, it's > more likely that the former will actually work :) > > > >> For Checkers that care about monitoring all of these cases, we > do have a Checker::VisitGenericCallExpr() that they can implement > > > >> > > > > Sure, but the same question crops up again: what defines the > simple case, and why? > >What defines the simple case is what I have seen from >experience. Most checkers just care about matching to simple names. > >My concern is about the ease of writing *correct* checkers with >minimal code. I want checker writers to always know what they >explicitly must be reasoning about, and no more. Defining a >restrictive interface in Checker allows checker writers to exactly >know what invariants they are coding to. > >I can buy the argument that checkers should reason about the general >case for names, and then explicitly match on what they care >about. From this design point, checkers should always use >NamedDecl::getDeclName(). Unfortunately, we vend 3 APIs for getting >the name of a NamedDecl: getIdentifier(), getNameAsString(), and >getDeclName(). The first seems very error prone (as we have seen), >the second is redundant, and the third really is the one API that >everyone should probably be using. > >I see two alternative options: > >(1) Have a restrictive interface in Checker, as I originally >proposed, to clearly limit the scope of what Checker writers must think about. > >(2) Remove NamedDecl::getIdentifier() and >NamedDecl::getNameAsString(). This forces all clients of NamedDecl >(both the analyzer and others) to explicitly reason about all cases. >While this may "add" code in some places, at least the compiler will >enforce that the client has explicitly reason about the the various cases.
Is the overlap between C, ObjectiveC and C++ a contributing factor? All of the experimental checks are grouped together at the moment having evolved from the original ObjectiveC memory checker. PR7287 arises because of checkers written for C running on C++ code. Is it time to split these and register only those checkers that are applicable to the specific code being analyzed and/or a specific option indicating which set of checks to use? Under that scenario, a pure C checker wouldn't need to worry about CXX expressions. If you want a checker to handle C++ as well as C, you need to aware of these issues. Checkers that start as pure C can be extended to work with C++ in the future. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
