[PATCH] D35258: [Plugins] Add a slim plugin API to work together with the new PM

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe added inline comments.


================
Comment at: include/llvm/Passes/PassPlugin.h:44
+
+#define LLVM_PLUGIN(NAME, PLUGIN_VERSION, REGISTER_CALLBACKS)                  \
+  extern "C" {                                                                 \
----------------
chandlerc wrote:
> Out of curiosity -- why use a macro rather than just documenting that a plugin must define the above function?
Mostly so that clients don't forget the extern "C". I just thought it was convenient and liked the declarative style.

If you have strong feelings about this I can document it instead, I'm not really married to the idea.


================
Comment at: include/llvm/Passes/PassPluginLoader.h:18
+namespace llvm {
+class Plugin {
+public:
----------------
chandlerc wrote:
> This whole class needs doxygen comments. =D
> 
> Also, should it be called PassPlugin? And maybe be in the same header as above?
I put the two classes in different headers because they have different clients. The `PassPlugin.h` header contains what's required on the client side, i.e. in the actual pass plugin. The `Plugin` class here is on the other hand only used by the driver to represent loaded plugins.


Repository:
  rL LLVM

https://reviews.llvm.org/D35258