> 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/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
