[cfe-dev] [PATCH] enable dynamic loading components for static analyzer

[Ted Kremenek]

On Nov 26, 2008, at 5:14 PM, Zhongxing Xu wrote:

> +/// 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>


Thanks Zhongxing.  A few comments:

===================================================================
--- include/clang/Analysis/PathSensitive/GRState.h	(revision 60131)
+++ include/clang/Analysis/PathSensitive/GRState.h	(working copy)
@@ -27,6 +27,7 @@
  #include "clang/AST/Decl.h"
  #include "clang/AST/ASTContext.h"
  #include "clang/Analysis/Analyses/LiveVariables.h"
+#include "clang/Driver/ManagerRegistry.h" // for typedefs.

This is a layering violation.  libAnalysis should not depend on  
libDriver, it should be the other way.  That's the whole point of  
putting ManagerRegistry in libDriver.  The typedefs belong in  
libAnalysis, either in GRState.h or in a new header file (one that is  
small).

Index: include/clang/Analysis/PathSensitive/GRExprEngine.h
===================================================================
--- include/clang/Analysis/PathSensitive/GRExprEngine.h	(revision 60131)
+++ include/clang/Analysis/PathSensitive/GRExprEngine.h	(working copy)
@@ -22,6 +22,7 @@
  #include "clang/Analysis/PathSensitive/GRTransferFuncs.h"
  #include "clang/AST/Type.h"
  #include "clang/AST/ExprObjC.h"
+#include "clang/Driver/ManagerRegistry.h" // for typedefs.

This is the same layering violation.  Have the typedefs be in  
libAnalysis, not libDriver.