On May 5, 2013, at 22:19 , Joshua Cranmer <[email protected]> wrote:
> On 5/3/2013 6:18 PM, Jordan Rose wrote:
>>>> 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.
>>
>> 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).
>
> 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.
I know. I think the general lack of interest from the community says a few
things about this: the general response to your new architecture has been
"mildly positive" or thereabouts, but nobody's been dying for this and so
nobody's shaping it in any significant way. Adding new UI that needs to be
future-proof should always be taken seriously
But on the other hand, you have had this up for a long time, and I can see how
this is a stepping-stone to more significant improvements (I like where the
custom C++11 attributes idea is going). So I won't block this, and I appreciate
you working on it.
>>> +Clang Plugins run a set of callbacks over code. These callbacks are set up
>>> by
>>> +the plugin when the compiler calls a function exported by the plugin. The
>>> main
>>> +function exported by the plugin is clang_plugin_begin_tu, which is called
>>> before
>>> +any file is compiled.
>>
>> s/any/each/, I think. Is there a separate callback that is guaranteed to
>> only be called once, or at least once per CompilerInstance?
>
> 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.
This is true of the Clang driver, but not necessarily true of a (hypothetical)
clang compilation or indexing server. Then again, leaving out the callback for
now doesn't preclude adding it later.
>>> +/// A struct containing sufficient information to express ABI
>>> compatibility for
>>> +/// plugins. It is sufficient for plugins to use the
>>> +/// DECLARE_PLUGIN_COMPATIBILITY macro to find compatibility.
>>> +struct Version {
>>> + /// The major version of clang (corresponds to CLANG_VERSION_MAJOR)
>>> + unsigned Major;
>>> + /// The minor version of clang (corresponds to CLANG_VERSION_MINOR)
>>> + unsigned Minor;
>>> +};
>>
>> VersionTuple is a bit overkill here, but do we really need another version
>> struct?
>
> 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.
Fair enough. I doubt VersionTuple is changing any time soon, but you make a
good point.
>>> + /// Arguments to the plugin
>>> + SmallVector<std::pair<std::string, std::string>, 4> Args;
>>
>> 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.
>
> I originally had this as a std::vector, and was told to make it a SmallVector
> instead.
Ha, okay.
Jordan
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
