On Jun 26, 2012, at 1:25 PM, Joshua Cranmer wrote:
> +def fperplugin_arg : Joined<"-fplugin-arg-">, Group<f_Group>,
> Flags<[CC1Option]>,
> + HelpText<"Pass an argument to a plugin">,
> MetaVarName<"<plugin>-<name>[=<value>]">;
It's really bugging me that the name of the flag doesn't match the typed form.
I think fplugin_arg is fine, even though there's more to it than that.
> +extern void clang_plugin_begin_file(llvm::StringRef FileName,
> + const clang::CompilerInstance &CI,
> + clang::plugin::PluginFileCallbacks
> &Callbacks);
I would still prefer clang_plugin_begin_tu, but maybe for consistency with the
rest of Clang your name is better.
> +class Plugin {
> + llvm::sys::DynamicLibrary PluginLibrary;
> + plugin::PluginFileCallbacks Callbacks;
> +public:
> + Plugin(llvm::sys::DynamicLibrary library) : PluginLibrary(library) {}
Still not initializing Callbacks to 0? "PluginLibrary(library), Callbacks()"
should do it just fine.
> + intptr_t getFunctionCall(const char *name) {
Nitpicking: you're not getting the /call/, you're getting the /function/. Or
even more precisely you're getting the function /address/.
> + if (ASTConsumer *Consumer = it->getCallbacks().astConsumer) {
> + Consumers.push_back(Consumer);
> + }
LLVM coding style says no braces around single statements in loops and
conditionals. This shows up in a couple of places.
> + std::vector<Plugin> Plugins;
Can this be a SmallVector?
> + for (unsigned i = 0,
> + E = Clang->getFrontendOpts().CompilerPlugins.size(); i != E; ++i) {
Recent convention uses "I" and "E"; "i" and "e" would be okay. I don't
understand "i" and "E". (Also, maybe you can just use iterators anyway? Not
sure if it'd be cleaner or not here.)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
