Page MenuHomePhabricator

[analyzer][NFC] Introduce CallDescriptionSets
ClosedPublic

Authored by steakhal on Wed, Nov 10, 10:45 AM.

Details

Summary

Sometimes we only want to decide if some function is called, and we
don't care which of the set.
This CallDescriptionSet will have the same behavior, except
instead of lookup() returning a pointer to the mapped value,
the contains() returns bool.
Internally, it uses the CallDescriptionMap<bool> for implementing the
behavior. It is preferred, to reuse the generic
CallDescriptionMap::lookup() logic, instead of duplicating it.
The generic version might be improved by implementing a hash lookup or
something along those lines.

Diff Detail

Event Timeline

steakhal created this revision.Wed, Nov 10, 10:45 AM
martong added a comment.EditedFri, Nov 12, 8:51 AM

I was thinking about that perhaps a higher level of abstraction would be useful.
Something like.

auto FM = FunctionMatcher{"foo", "bar"};
//....
if (FM.matches(CallEvent))
  //...
using FunctionMatcher = CallDescriptionMap<void>;

?

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
123

Could you please add some documentation that this could be used if the user only want to decide if some function is called/matched?

I was thinking about that perhaps a higher level of abstraction would be useful.
Something like.

auto FM = FunctionMatcher{"foo", "bar"};
//....
if (FM.matches(CallEvent))
  //...
using FunctionMatcher = CallDescriptionMap<void>;

I don't know. I don't really want to introduce yet another entity. They do the same basically. You can convince me though.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
123

Great point! I'll.

Szelethus requested changes to this revision.Mon, Nov 15, 4:04 AM

I recognize the problem, but don't agree with the solution: this is a CallDescriptionSet at this point, not a CallDescriptionMap. Also, I just don't think this is the place to use template specialization, nor do I think we should keep the function name lookup, if we only care about whether the call is in the set or not.

This revision now requires changes to proceed.Mon, Nov 15, 4:04 AM
steakhal updated this revision to Diff 388426.Fri, Nov 19, 2:12 AM
steakhal marked an inline comment as done.
steakhal retitled this revision from [analyzer][NFC] Specialize CallDescriptionMaps for void to [analyzer][NFC] Introduce CallDescriptionSets.
steakhal edited the summary of this revision. (Show Details)
  • define the CallDescriptionSet class, by wrapping a CallDescriptionMap
  • add doc comments about the class

I recognize the problem, but don't agree with the solution: this is a CallDescriptionSet at this point, not a CallDescriptionMap. Also, I just don't think this is the place to use template specialization, nor do I think we should keep the function name lookup, if we only care about whether the call is in the set or not.

You are right! Fixed by now.

martong accepted this revision.Fri, Nov 19, 2:52 AM

Very good! LGTM!

This revision is now accepted and ready to land.Fri, Nov 19, 6:08 AM
This revision was landed with ongoing or failed builds.Fri, Nov 19, 9:33 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFri, Nov 19, 9:33 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript