[PATCH] libclang plugin patch

Manuel Klimek klimek at google.com
Mon Oct 6 00:57:30 PDT 2014


Thanks a lot for working on this!
1. Some general high level feedback inline
2. Can you please upload the next patch with full context (http://llvm.org/docs/Phabricator.html)
With svn, you do that by typing:
$ svn diff --diff-cmd=diff -x -U999999

================
Comment at: examples/PrintFunctionNames/CMakeLists.txt:13
@@ -12,2 +12,3 @@
 add_llvm_loadable_module(PrintFunctionNames PrintFunctionNames.cpp)
+target_link_libraries( PrintFunctionNames libclang )
 
----------------
The other code around doesn't seem to have spaces in parenthesis. The general guideline in llvm is to be consistent with the code around you - that makes reviewing much more pleasurable ;)

================
Comment at: examples/PrintFunctionNames/PrintFunctionNames.cpp:26-42
@@ -25,7 +25,19 @@
 public:
+  PrintFunctionsConsumer( const CompilerInstance& _CI ):
+    CI(_CI)
+  {
+    
+  }
+  const CompilerInstance& CI;
   virtual bool HandleTopLevelDecl(DeclGroupRef DG) {
+    DiagnosticsEngine& DE = CI.getDiagnostics();
     for (DeclGroupRef::iterator i = DG.begin(), e = DG.end(); i != e; ++i) {
       const Decl *D = *i;
-      if (const NamedDecl *ND = dyn_cast<NamedDecl>(D))
+      if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)){
         llvm::errs() << "top-level-decl: \"" << ND->getNameAsString() << "\"\n";
+	unsigned DiagID = DE.getCustomDiagID(DiagnosticsEngine::Error,
+	    "functions are bad '%0'");
+	DE.Report( ND->getLocStart(), DiagID ) 
+	  << ND->getNameAsString();
+      }
     }
----------------
Can you clang-format over it.

================
Comment at: examples/PrintFunctionNames/PrintFunctionNames.cpp:31
@@ -26,1 +30,3 @@
+  }
+  const CompilerInstance& CI;
   virtual bool HandleTopLevelDecl(DeclGroupRef DG) {
----------------
I think having a field in the middle between methods leads to it getting lost. Also, does this have to be public?

There are basically 2 ways in llvm/clang to sort classes:
Either do it Java-style (private members first, then public methods), or the style I prefer that has all public methods up top, and then a private: section with private variables in the end. In each section, be consistent about how you group your methods and members; I personally like to have the methods first, then the fields, but generally, be consistent, and have some visible structure.

http://reviews.llvm.org/D5611






More information about the cfe-commits mailing list