libclang plugin patch

[realincubus]

Summary

The goal is to enable tools that use libclang to parse c/c++ code to display additional errors and warnings emitted by plugins.
Normally libclang does not load any plugins when its parsing code through clang_parseTranslationUnit.
This patch moves the plugin loading code from ExecuteCompilerInvocation to the class CompilerInstance and
adds the method calls to ASTUnit::Parse and ExecuteCompilerInvocation.
I additionally added the symbols that are needed for the plugin to link against libclang to tools/libclang/libclang.exports.
If one links the plugin against libclang it works from the vim plugin YouCompleteMe and clang itself.

[klimek]

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

examples/PrintFunctionNames/CMakeLists.txt

13

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 ;)

examples/PrintFunctionNames/PrintFunctionNames.cpp

26–42

Can you clang-format over it.

31

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.

[realincubus]

last patch was against the wrong revision.

[klimek]

With the nits fixed, it generally looks good from my side. The question is who the right owner is to give the good to go. Looping in Richard to delegate to the right person.

examples/PrintFunctionNames/PrintFunctionNames.cpp

26

No need for the _. CI(CI) works fine (and does TheRightThing).

include/clang/Frontend/CompilerInstance.h

713

Please make comments start with a capital letter.
Also, new doxygen comments should start with \brief
/// \brief Loads plugins that were added by -load.

lib/Frontend/CompilerInstance.cpp

1626

This comment seems superfluous :)

lib/FrontendTool/ExecuteCompilerInvocation.cpp

182

As does this now.

[klimek]

Looping in people who can find the right approver for this.

[chandlerc]

I'm definitely not the right reviewer for this. Richard or Doug make much more sense.

[klimek]

What's the state of this?

[klimek]

Sounds good!

examples/PrintFunctionNames/CMakeLists.txt

 endif()
 
 add_llvm_loadable_module(PrintFunctionNames PrintFunctionNames.cpp)
+target_link_libraries( PrintFunctionNames libclang )
 
 if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
   target_link_libraries(PrintFunctionNames ${cmake_2_8_12_PRIVATE}

examples/PrintFunctionNames/PrintFunctionNames.cpp

 
 class PrintFunctionsConsumer : public ASTConsumer {
 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();
+      }
     }
 
     return true;

 protected:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  llvm::StringRef) {
-    return llvm::make_unique<PrintFunctionsConsumer>();
+    return llvm::make_unique<PrintFunctionsConsumer>(CI);
   }
 
   bool ParseArgs(const CompilerInstance &CI,

include/clang/Frontend/CompilerInstance.h

   void addDependencyCollector(std::shared_ptr<DependencyCollector> Listener) {
     DependencyCollectors.push_back(std::move(Listener));
   }
+
+  /// load plugins that were added by -load
+  void loadPlugins();
 };
 
 } // end namespace clang

lib/Frontend/ASTUnit.cpp

   llvm::CrashRecoveryContextCleanupRegistrar<TopLevelDeclTrackerAction>
     ActCleanup(Act.get());
 
+  Clang->loadPlugins();
+
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
     goto error;
 

lib/Frontend/CompilerInstance.cpp

 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include <sys/stat.h>
 #include <system_error>
 #include <time.h>

   return false;
 }
 void CompilerInstance::resetAndLeakSema() { BuryPointer(takeSema()); }
+
+void 
+CompilerInstance::loadPlugins(){
+  // Load any requested plugins.
+  for (unsigned i = 0, e = getFrontendOpts().Plugins.size(); i != e; ++i) {
+    const std::string &Path = getFrontendOpts().Plugins[i];
+    std::string Error;
+    if (llvm::sys::DynamicLibrary::LoadLibraryPermanently(Path.c_str(), &Error))
+      getDiagnostics().Report(diag::err_fe_unable_to_load_plugin)
+        << Path << Error;
+  }
+}
+

lib/FrontendTool/ExecuteCompilerInvocation.cpp

   }
 
   // Load any requested plugins.
-  for (unsigned i = 0,
-         e = Clang->getFrontendOpts().Plugins.size(); i != e; ++i) {
-    const std::string &Path = Clang->getFrontendOpts().Plugins[i];
-    std::string Error;
-    if (llvm::sys::DynamicLibrary::LoadLibraryPermanently(Path.c_str(), &Error))
-      Clang->getDiagnostics().Report(diag::err_fe_unable_to_load_plugin)
-        << Path << Error;
-  }
+  Clang->loadPlugins();
 
   // Honor -mllvm.
   //

tools/libclang/libclang.exports

 clang_VirtualFileOverlay_dispose
 clang_VirtualFileOverlay_setCaseSensitivity
 clang_VirtualFileOverlay_writeToBuffer
+_ZN5clang15PluginASTAction6anchorEv
+_ZN4llvm11raw_ostream5writeEPKcm
+_ZN4llvm4errsEv
+_ZN5clang11ASTConsumer33HandleTopLevelDeclInObjCContainerENS_12DeclGroupRefE
+_ZN5clang14FrontendActionD2Ev
+_ZN5clang13DiagnosticIDs15getCustomDiagIDENS0_5LevelEN4llvm9StringRefE
+_ZN5clang17ASTFrontendAction13ExecuteActionEv
+_ZN5clang11ASTConsumer24HandleImplicitImportDeclEPNS_10ImportDeclE
+_ZN5clang14FrontendAction22shouldEraseOutputFilesEv
+_ZNK5clang15DeclarationName11getAsStringEv
+_ZN5clang17DiagnosticsEngine21EmitCurrentDiagnosticEb
+_ZN5clang11ASTConsumer21HandleInterestingDeclENS_12DeclGroupRefE
+_ZN5clang14FrontendActionC2Ev
+_ZN4llvm8RegistryIN5clang15PluginASTActionENS_14RegistryTraitsIS2_EEE4nodeC1ERKNS_19SimpleRegistryEntryIS2_EE
+_ZTVN5clang15PluginASTActionE
+_ZTVN5clang17ASTFrontendActionE
+_ZTVN5clang11ASTConsumerE