[PATCH] [Review request][analyzer]Information about special handling for a particular region/symbol.

Anna Zaks ganna at apple.com
Tue Sep 24 13:39:46 PDT 2013


I don't see any replies to the comments. 

Looks like the first two comments are not addressed :

================
Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:368-370
@@ -367,3 +367,5 @@
  ///        parameters to the given call.
-  /// \param IsConst Specifies if the pointer is const.
+  /// \param Kind The reason of pointer escape.
+  /// \param HTraits Information about special handling for a particular 
+  ///        region/symbol.
  /// \returns Checkers can modify the state by returning a new one.
----------------
I know this is not new to this patch, but PointerEscapeKind and the traits info are very related...

Would it make sense to extend the kind enum and use it for traits? One benefit I see is that all the info about the escape would be in one place. We would only have one checker callback.. On the other hand, I don't recall why they are different concepts now..

================
Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:377
@@ -374,3 +376,3 @@
                              PointerEscapeKind Kind,
-                              bool IsConst = false);
+                              RegionAndSymHandlingTraits *HTraits = NULL);

----------------
When does it make sense for traits to be missing? (Why the NULL initialization? Also, I think it should be '0', not "NULL")

Thanks,
Anna.
On Sep 24, 2013, at 12:44 PM, Anton Yartsev <anton.yartsev at gmail.com> wrote:

> On 24.09.2013 21:09, Anna Zaks wrote:
>> Anton,
>> 
>> Does the new patch address all my comments? I've looked at it briefly and it does not seem to be the case..
>> 
>> Anna.
> Yes, it does. Did you see my replays on your comments?
> http://llvm-reviews.chandlerc.com/differential/diff/3700/
>> 
>> On Sep 16, 2013, at 6:55 PM, Антон Ярцев <anton.yartsev at gmail.com> wrote:
>> 
>>>  Attached is an updated patch.
>>> 
>>> Hi jordan_rose, zaks.anna,
>>> 
>>> http://llvm-reviews.chandlerc.com/D1486
>>> 
>>> CHANGE SINCE LAST DIFF
>>>  http://llvm-reviews.chandlerc.com/D1486?vs=3700&id=4331#toc
>>> 
>>> Files:
>>>  include/clang/StaticAnalyzer/Core/CheckerManager.h
>>>  include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
>>>  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
>>>  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
>>>  include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
>>>  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
>>>  include/clang/StaticAnalyzer/Core/Checker.h
>>>  lib/StaticAnalyzer/Core/ProgramState.cpp
>>>  lib/StaticAnalyzer/Core/CallEvent.cpp
>>>  lib/StaticAnalyzer/Core/CheckerManager.cpp
>>>  lib/StaticAnalyzer/Core/ExprEngine.cpp
>>>  lib/StaticAnalyzer/Core/RegionStore.cpp
>>> <D1486.2.patch>_______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 
> -- 
> Anton
> 





More information about the cfe-commits mailing list