This is an archive of the discontinued LLVM Phabricator instance.

[clang][dataflow] Model the behavior of non-standard optional assignment
ClosedPublic

Authored by sgatev on Mar 16 2022, 3:21 PM.

Details

Summary

Model nullopt, value, and conversion assignment operators.

Diff Detail

Event Timeline

sgatev created this revision.Mar 16 2022, 3:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 3:21 PM
sgatev requested review of this revision.Mar 16 2022, 3:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 3:21 PM
xazax.hun added inline comments.Mar 16 2022, 3:43 PM
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
94

While really like the convenience matchers will give us building data flow analyses, I wonder whether a lot of duplicated work is happening in the background. E.g. will we end up string comparing the class name of the parent of the method in the matcher built by optionalClass whenever we process an overloaded operator=?

In a handwritten implementation we would only do this once, store the address of the canonical declaration somewhere and subsequent checks would only look up the canonical declaration of the called method and do a pointer comparison.

In case this matcher approach is not that efficient, I wonder if it would be possible to come up with some APIs where the matching is only done once and subsequent transfer functions would only use the cached pointers to declarations/types. In case the matchers are already doing something smart in the background feel free to ignore this comment.

173–174

If we never assign values to Parens, shouldn't getValue do the skipping instead?

296

Is this duplicated across here and transferValueOrConversionConstructor? Should we have a separate function like IsValueOperation?

sgatev updated this revision to Diff 416132.Mar 17 2022, 4:06 AM
sgatev marked 3 inline comments as done.

Address comments.

sgatev added inline comments.Mar 17 2022, 4:09 AM
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
94

That's a great point! Unfortunately, I'm not familiar with the implementation of matchers so I'll need to take a closer look/ask around to understand if such an optimization is already in place. Let me take a note of this for now. I suggest revising the APIs once we know more about the internals of matchers.

xazax.hun accepted this revision.Mar 17 2022, 9:03 AM
This revision is now accepted and ready to land.Mar 17 2022, 9:03 AM