xazax.hun added inline comments.

================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:513
+  const RecordDecl *RD = BaseTy->getDecl();
+  if (RD->getIdentifier() == nullptr || RD->getName() != "Message")
+    return false;
----------------
ymandel wrote:
> xazax.hun wrote:
> > Not sure how often is this invoked but we could reduce the number of string 
> > comparisons by caching the identifier ptr and do a pointer comparison.
> Good question. It means an extra comparison for each type until the pointer 
> is cached (to check if the cache is set) and then, afterwards, 2 comparisons 
> vs ~10 for the common case where the class name is doesn't match. In the 
> matching case, though, it is clearly saving much more.
> 
> For proto-heavy code, it seems a win, and a loss otherwise.  But, the 
> question is where to put the cache. It seems to me best to move this to be a 
> method on DataflowAnalysisContext (since it is a global, not local env, 
> property) and make the cached pointer a private member of DAC.
> 
> Thoughts?
I think it might be nice to have a context that is scoped to the translation 
unit rather than a function. We might have other stuff that we want to cache 
here.

An example how the static analyzer is dealing with this: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp#L960

In case we do not want to be lazy and willing to populate the cache eagerly at 
the beginning of the analysis, we would not pay for the extra checks to see if 
the cache is populated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123032

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to