[PATCH] D121863: [clang][dataflow] Model the behavior of non-standard optional assignment

Thu Mar 17 04:09:08 PDT 2022

sgatev added inline comments.

================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:81

+auto isOptionalValueOrConversionAssignment() {
+  return cxxOperatorCallExpr(
----------------
xazax.hun wrote:
> 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. 
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.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121863/new/

https://reviews.llvm.org/D121863