[cfe-dev] [PATCH] Implement a sane plugin API for clang

[Joshua Cranmer]

On 3/14/2013 3:42 PM, Sean Silva wrote:
> On Thu, Mar 14, 2013 at 12:30 AM, Joshua Cranmer <pidgeot18 at gmail.com 
> <mailto:pidgeot18 at gmail.com>> wrote:
>
>     On 3/13/2013 10:05 PM, Sean Silva wrote:
>
>         While I'm all for the idea of improving the plugin API, I
>         think that a modest reduction in boilerplate is not
>         sufficiently compelling to foist a new plugin API on people
>         who already have existing code. The funny thing about
>         boilerplate is that it's easy to copy-paste, so it doesn't
>         really impede people from achieving their goals since they can
>         just copy the code that already works. The primary problem of
>         boilerplate is that it has the effect of deterring newbies,
>         and that issue can be easily combated with improved
>         documentation, which avoids breaking every external plugin and
>         tutorial on plugins.
>
>
>     One thing that REALLY sucks with the current approach is the need
>     to specify clang -Xclang -load -Xclang <plugin tarball> -Xclang
>     -add-plugin <plugin name> -Xclang -plugin-<name>-arg -Xclang
>     <blah> ...
>     With the new approach, the command line is clang
>     -fplugin=<tarball> -fplugin-arg-<name>-<arg>=<blah>, which is a
>     much shorter command line and can actually be passed into
>     CFLAGS/CXXFLAGS without driving libtool bonkers (I detest the need
>     for wrapper scripts just to pass arguments) and also eliminates
>     warnings whenever people use $(CXX) $(CXXFLAGS) as the linker.
>
>
>
> Realistically a tiny script might be a better long-term design, 
> allowing e.g. "clang++ `clang-plugin-config myPlugin.so arg1 arg2` 
> foo.cpp". Remember that the primary advantage of plugins vs 
> libtooling/libclang is that they are run as part of a build process, 
> meaning that in reality these command lines are meant to be generated 
> by a "configure" step and not by hand. So really the "user 
> friendliness" is determined by "how easy is it to integrate a clang 
> plugin into my build", and not by the exact commandline syntax per se. 
> This kind of script could also serve as a useful layer of indirection 
> and "user friendliness", e.g. it could recognize a "CLANG_PLUGIN_PATH" 
> or other niceties that would be dubious to add to clang itself.

A "tiny script" sounds to me a much worse option: you now have to run 
two processes per compiler invocation instead of one (and on Windows, 
process creation can take hundreds of milliseconds, so it's a noticeable 
slowdown). Also, you have to find more binaries to run it: if I specify 
CXX via a path, how should a build system know where to run 
clang-plugin-config from? You could guess by looking up the dirname of 
CXX and hoping it's there, but you are also advocating using shell 
scripts to represent CXX in another email, which renders this approach 
impossible.

> If you really want to immediately push plugins forward in a big way, 
> it would be monumental to set up a buildbot that runs a clang plugin 
> that does extra checking that isn't really appropriate for being 
> integrated as a diagnostic into the compiler proper. For example, a 
> plugin that warns on incorrect uses of dyn_cast<>. For maximum effect 
> this should be developed in-tree (probably in clang-tools-extra. Even 
> though it has "tools" in the name, I don't think anybody would be 
> opposed to developing plugins in there). It should also have an easy 
> way for people in our community to come up with and implement good 
> extra checks and get them integrated into that buildbot.

I am working on adding a compiler static checker plugin to Mozilla that 
would check the guarantees our old dehydra plugin used to check: a "must 
override" annotation (all subclasses must provide their own 
implementation of this method), a "stack class" annotation (this class 
cannot be allocated from the heap), and a warning that gets emitted 
every time you emit a static initializer.

>
>     The changes in this patch retain almost all of the same
>     functionality as the original plugin approach (including the
>     ability to do things like add custom compile passes via the
>     RegisterPass statics) while wrapping it in a much saner wrapper.
>
>
> My opposition to the current patch is that it does not provide enough 
> value to our users to compensate for the inconvenience that it will 
> cause them (by breaking their code). My opposition is not technical; I 
> don't doubt that your approach here is an improvement from a purely 
> technical standpoint.

The current plugin approach presumes that it is a pure consumer of the 
AST, which isn't a viable option in my opinion. One thing I would like 
to do in the future is be able to map Decls in the AST to functions 
emitted in the LLVM IR, which is completely impossible under the current 
architecture. Note also that I'm not removing the current (more or less 
broken) plugin architecture, so I'm not compelling people to switch. 
Rather, this is about enabling future changes that permit plugins to not 
take the view that they happen independently of code generation.

-- 
Joshua Cranmer
News submodule owner
DXR coauthor

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130315/66e0eef1/attachment.html>