[PATCH] D105553: [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract NoStateChangeVisitor class
Artem Dergachev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 9 12:29:47 PDT 2021
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
This looks perfectly sensible to me.
In the mailing list I seem to have made a mistake about how this works: we don't explicitly scan the AST for potential statements that could cause a state change (eg., assignment operators in case of `NoStore`) but we only check if the region was explicitly made accessible. This makes things a lot easier (we don't have to build an auxiliary AST-based analysis) and I'm not sure if this other heuristic that I thought we have would actually make things significantly better. I guess it makes sense to keep in mind that we might want to ultimately plug it in but I don't immediately see what'd stop us.
================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:629
+
+/// Put a diagnostic on return statement of all inlined functions for which some
+/// property remained unchanged.
----------------
Or on the `}` if there's no return statement.
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:399
+/// to get the correct parameter name.
+static ArrayRef<ParmVarDecl *> getCallParameters(const CallEvent &Call) {
+ // Use runtime definition, if available.
----------------
While we're at it, can you take a look if the recently introduced `CallEvent::parameters()` is good enough for this?
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:532-533
- /// Check and lazily calculate whether the region of interest is
- /// modified in the stack frame to which \p N belongs.
- /// The calculation is cached in FramesModifyingRegion.
- bool isRegionOfInterestModifiedInFrame(const ExplodedNode *N) {
- const LocationContext *Ctx = N->getLocationContext();
- const StackFrameContext *SCtx = Ctx->getStackFrame();
- if (!FramesModifyingCalculated.count(SCtx))
- findModifyingFrames(N);
- return FramesModifyingRegion.count(SCtx);
- }
+ // Region of interest corresponds to an IVar, exiting a method
+ // which could have written into that IVar, but did not.
+ virtual PathDiagnosticPieceRef
----------------
I guess this comment could be copied to the other two virtual methods?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105553/new/
https://reviews.llvm.org/D105553
More information about the cfe-commits
mailing list