[PATCH] [Analyzer] Individual options for checkers #2

[Anna Zaks]

> Right now there is no way for a checker to specify the package or checker name when querying an option. The getCheckerOption function is private 

> (the config table is public tough). The checkers can use getBooleanOption with a pointer to the checker, this means filling the package name and >checker name is automatic. With this API a checker can not query the options of an unrelated package and there is no way to misspell the package or 

> the checker name.


The model I was going for was simpler. A checker would be allowed to query options that are set on itself or a given package. For that, we would want to expose the string based APIs to the checkers. However, this solution is good as well and does have a benefit of being less error prone. The only downside that I see with this solution is that a checker will not be able to search options of other checkers/packages. Maybe it's fine to disallow that. What do you think?

I would like to stay away from mentioning "inheritance" in these APIs. Here is a summary of the thread where this was discussed (on the "Static Analyzer Checker Options Proposal" thread):

Anna (initial proposal):
"However, there will be no inheritance (i.e. the setting 'unix:Optimistic' is entirely distinct from the setting ‘unix.Malloc:Optimistic’)."
Gabor Kozar:
"I think inheritance like that could be useful in some situations."
Anna:
"The main reason is that we currently do not have a need for it and allowing inheritance requires a design for it. For example, what happens when a package adds an option? How would the checkers access it? If we were to allow dynamically loaded checkers how would they inherit it? What happens if a checker overrides a package option?"
Gabor Kozar:
"I would expect such options to be inherited, i.e. if I set 'unix:Optimistic', then I expect this to be visible everywhere in the 'unix' package. Why are disallowing this?"
Anna:
"The package options will be visible to checkers; specifically, checkers could query the package options. For example, MallocChecker could call getOption(Optimistic, "unix") to check if the option has been set on the package. "
Gabor Kozar:
"How about the getOption() function getting a boolean parameter specifying whether to allow "inherited" options, with the default being true?

getOption("unix.stuff.moo.FooChecker", "MyOption", true) would then be equivalent to trying to all of these:
unix.stuff.moo.FooChecker:MyOption
unix.stuff.moo:MyOption
unix.stuff:MyOption
unix:MyOption
I think this behavior would be clear and straight-forward."
Anna:
"This looks good to me. I was mainly thinking/talking about implicit inheritance. Improving the way checkers can query options is a definite plus."

Given the above, the name "OptionKind" is confusing. (It implies that options have kinds, but we don't specify/enforce option kinds anywhere. We don't have a notion of an inherited option that will be automatically applied to all checkers in that package.) It's not an option kind but rather a search mode requested by the checker. It could be as easy as just a bool - search for the option in all parent packages or only search in this checker. In this patch, you allow many more ways of searching (OptionKind), which is fine if well documented and tested. However, if no one needs to all of these, implementing them could be an overkill at this point. It also complicated the API. We also need to document what happens when an option is overriden. The more kinds we have, the more complex the rule will be.

> I think the most appropriate way to test this feature would be to write some unit tests for AnalyzerOptions class. However there are no 

>  unittests for the static analyzer yet. Should I create some as part of this patch?


If you volunteer to do this, that would be awesome! There is no good way of testing these now.


http://reviews.llvm.org/D7905

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/