<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 5/3/2013 6:18 PM, Jordan Rose wrote:<br>
</div>
<blockquote
cite="mid:EF146988-CF53-4A2C-8194-D17CC7DA1E2F@apple.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<div>
<blockquote type="cite" style="font-family: Menlo-Regular;
font-size: 11px;">
<blockquote type="cite">We really ought to have a preliminary
discussion on cfe-dev to map out the directions we want to
take the plugin API, and what *compelling*, *enabling*
changes we can make to it.<br>
</blockquote>
</blockquote>
<br>
</div>
<div>While I mostly like what you've come up with, Sean has a
point that the community hasn't really picked it up. I have your
e-mail from a year ago still flagged ("[cfe-dev] Thinking about
clang plugins"). No one responded to it then, but I think it was
a good summary of the problems you were/are trying to solve. I
would say to bring that up again...except most likely nothing
will happen until after the 3.3 release (and for us Apple folks,
after WWDC).</div>
</blockquote>
<br>
I had originally wanted to get this patch into the 3.1 release. That
nothing can come before 3.3 is... disheartening, to say the least.<br>
<blockquote
cite="mid:EF146988-CF53-4A2C-8194-D17CC7DA1E2F@apple.com"
type="cite">
<blockquote type="cite">
<div>
<div>+Clang Plugins run a set of callbacks over code. These
callbacks are set up by</div>
<div>+the plugin when the compiler calls a function exported
by the plugin. The main</div>
<div>+function exported by the plugin is
clang_plugin_begin_tu, which is called before</div>
<div>+any file is compiled.</div>
</div>
</blockquote>
<div><br>
</div>
<div>s/any/each/, I think. Is there a separate callback that is
guaranteed to only be called once, or at least once per
CompilerInstance?</div>
</blockquote>
<br>
If you call clang a.cpp b.cpp, the last time I checked, this spawns
two clang -cc1 processes. Since the plugin is loaded in the clang
-cc1 process, it gets loaded twice, so call-per-invocation is
effectively equivalent to call-per-translation unit. My original
implementation had a call-per-invocation callback, but I removed it
after discovering this fact since its effect were misleading.<br>
<br>
<blockquote
cite="mid:EF146988-CF53-4A2C-8194-D17CC7DA1E2F@apple.com"
type="cite">
<blockquote type="cite">
<div>
<div>+/// A struct containing sufficient information to
express ABI compatibility for</div>
<div>+/// plugins. It is sufficient for plugins to use the</div>
<div>+/// DECLARE_PLUGIN_COMPATIBILITY macro to find
compatibility.</div>
<div>+struct Version {</div>
<div>+ /// The major version of clang (corresponds to
CLANG_VERSION_MAJOR)</div>
<div>+ unsigned Major;</div>
<div>+ /// The minor version of clang (corresponds to
CLANG_VERSION_MINOR)</div>
<div>+ unsigned Minor;</div>
<div>+};</div>
</div>
</blockquote>
<div><br>
</div>
<div>VersionTuple is a bit overkill here, but do we really need
another version struct?</div>
</blockquote>
<br>
My main rationale for making a new version struct was that this
would be effectively ABI-frozen. Since the version code is used to
decide whether or not to attempt to use the plugin, an ABI-mismatch
is particularly deadly for the compiler in this code.<br>
<br>
<blockquote
cite="mid:EF146988-CF53-4A2C-8194-D17CC7DA1E2F@apple.com"
type="cite">
<blockquote type="cite">
<div>All non-null callbacks will be deleted by the compiler
internally.</div>
</blockquote>
<div><br>
</div>
<div>"On return, the compiler takes ownership of any non-null
callbacks."</div>
<div><br>
</div>
<div>...although I'm not sure this is a good idea. Why not reuse
the same callbacks over multiple TUs?</div>
</blockquote>
<br>
As mentioned earlier, in practice, a single invocation gets called
for a single TU.<br>
<blockquote
cite="mid:EF146988-CF53-4A2C-8194-D17CC7DA1E2F@apple.com"
type="cite">
<blockquote type="cite">
<div>
<div>+ /// Arguments to the plugin</div>
<div>+ SmallVector<std::pair<std::string,
std::string>, 4> Args;</div>
</div>
</blockquote>
<div><br>
</div>
<div>Should this just be a std::vector? I think the most common
case will be 0 args, in which case SmallVector doesn't do much
good.</div>
</blockquote>
<br>
I originally had this as a std::vector, and was told to make it a
SmallVector instead.<br>
<br>
<blockquote
cite="mid:EF146988-CF53-4A2C-8194-D17CC7DA1E2F@apple.com"
type="cite">...any reason not to do this with a map, to avoid N
iterations over the arguments?</blockquote>
<br>
There is likely to be very few arguments and plugins in use for a
given invocation; with such small numbers, the extra overhead of a
map isn't worth it.<br>
<br>
<pre class="moz-signature" cols="72">--
Joshua Cranmer
News submodule owner
DXR coauthor</pre>
</body>
</html>