This is an archive of the discontinued LLVM Phabricator instance.

[analyzer][NFC] Introduce CallDescriptionSets
ClosedPublic

Authored by steakhal on Nov 10 2021, 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.Nov 10 2021, 10:45 AM
martong added a comment.EditedNov 12 2021, 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.Nov 15 2021, 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.Nov 15 2021, 4:04 AM
steakhal updated this revision to Diff 388426.Nov 19 2021, 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.Nov 19 2021, 2:52 AM

Very good! LGTM!

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