This is an archive of the discontinued LLVM Phabricator instance.

[analyzer][Casting] Support isa, cast, dyn_cast of SVals
ClosedPublic

Authored by steakhal on May 16 2022, 10:35 AM.

Details

Summary

This change specializes the LLVM RTTI mechanism for SVals.
After this change, we can use the well-known isa, cast, dyn_cast.

Examples:

// SVal V = ...;
// Loc MyLoc = ...;

bool IsInteresting = isa<loc::MemRegionVal, loc::GotoLabel>(MyLoc);
auto MRV = cast<loc::MemRegionVal>(MyLoc);
Optional<loc::MemRegionVal> MaybeMRV = dyn_cast<loc::MemRegionVal>(V)

The current SVal::getAs and castAs member functions are redundant at
this point, but I believe that they are still handy.

The member function version is terse and reads left-to-right, which IMO
is a great plus. However, we should probably add a variadic isa member
function version to have the same casting API in both cases.

I'm not sure, shall I add tests?

Thanks for the extensive TMP help @bzcheeseman!

Diff Detail

Event Timeline

steakhal created this revision.May 16 2022, 10:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2022, 10:35 AM
steakhal requested review of this revision.May 16 2022, 10:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2022, 10:35 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
steakhal edited the summary of this revision. (Show Details)May 16 2022, 10:38 AM
bzcheeseman added inline comments.May 16 2022, 10:41 AM
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
100–101

Is it worth refactoring this and getAs to use cast<T> and dyn_cast<T> as appropriate to avoid code duplication?

steakhal updated this revision to Diff 429947.May 17 2022, 12:37 AM

Use cast and dyn_cast in castAs and getAs respecitvely.

steakhal marked an inline comment as done.EditedMay 17 2022, 12:38 AM

I had to fix the doCast to return To instead of Optional<To> to make it work.

I'm not sure, shall I add tests?

Yes, please. Unit tests for dyn_cast and isa should be easy. However, I am not sure how to test cast for the failure cases.

bzcheeseman added a comment.EditedMay 17 2022, 7:01 AM

I had to fix the doCast to return To instead of Optional<To> to make it work.

That's fine (or it should be!), you could dereference the optional if you wanted to

I'm not sure, shall I add tests?

Yes, please. Unit tests for dyn_cast and isa should be easy. However, I am not sure how to test cast for the failure cases.

What if we were using the 'new' casting style at some places. If it compiles, it should be fine.
It feels weird to check stuff, only in static_asserts.

I had to fix the doCast to return To instead of Optional<To> to make it work.

That's fine (or it should be!), you could dereference the optional if you wanted to

Currently, we expect that casts result in regular SVal objects, instead of pointer-like objects , thus this code to compile:
NonLoc N = llvm::cast<NonLoc>(V), where V is of type SVal. I believe that is why I decided to make that change.

bzcheeseman accepted this revision.May 17 2022, 7:26 AM

I had to fix the doCast to return To instead of Optional<To> to make it work.

That's fine (or it should be!), you could dereference the optional if you wanted to

Currently, we expect that casts result in regular SVal objects, instead of pointer-like objects , thus this code to compile:
NonLoc N = llvm::cast<NonLoc>(V), where V is of type SVal. I believe that is why I decided to make that change.

Makes sense, LGTM

This revision is now accepted and ready to land.May 17 2022, 7:26 AM

Thanks for your help @bzcheeseman!

While not having tests might be OK, but I'd prefer to introduce at least a couple uses of the new facilities so existing tests cover them.

steakhal updated this revision to Diff 430327.May 18 2022, 4:55 AM
  • Added uses for dyn_cast and cast of the new specialization.

While not having tests might be OK, but I'd prefer to introduce at least a couple uses of the new facilities so existing tests cover them.

I've added a use of the new dyn_cast and cast free function specializations.

This revision was automatically updated to reflect the committed changes.