This patch enables dynamically loaded components for static analyzer. It uses global objects of Register* classes. More comments in the patch.
load.patch (7.33 KB)
This patch enables dynamically loaded components for static analyzer. It uses global objects of Register* classes. More comments in the patch.
load.patch (7.33 KB)
Looks great! I'm really excited about this. My biggest meta point is that I feel that ManagerRegister and RegisterConstraintManager should go in libDriver, not libAnalysis (feel free to comment about this opinion). Specific comments inline:
Index: include/clang/Analysis/PathSensitive/GRExprEngine.h
+/// ManagerRegister - This class records manager creators registered at
+/// runtime. The information is communicated to AnalysisManager through static
+/// members. Better design is expected.
+
+class ManagerRegister {
+public:
- typedef ConstraintManager* (*ConstraintManagerCreator)(GRStateManager&);
- typedef StoreManager* (*StoreManagerCreator)(GRStateManager&);
Aren’t these typedefs the same as in the ones in GRStateManager? Maybe we should just move those typedefs into the clang namespace and have them used by both GRStateManager and ManagerRegister (if not, let’s just keep the ones in GRStateManager). It seems silly to have the same typedefs defined twice, which can lead to weird warnings if we don’t keep them in sync.
Done. But this introduced two #includes, commented in code. And I noticed there are other #includes only for including typedefs, for example LiveSymbolsTy/DeadSymbolsTy.
I named the class ManagerRegistry instead of ManagerRegistrar.
Other comments were adopted. Thanks.
New patch attached.
load2.patch (7.72 KB)
Thanks Zhongxing. A few comments:
Looks great. Please apply.