Folks on cfe-dev seem happy with this direction, so go ahead and submit with a tweaked comment as below...
================ Comment at: include/clang/Driver/ToolChain.h:166-171 @@ -165,3 +165,8 @@ /// Choose a tool to use to handle the action \p JA. - Tool *SelectTool(const JobAction &JA) const; + /// This is virtualized because there are scenarios where you want the + /// default driver in the default driver-mode not to pick Clang as + /// the right tool for a C program, at the discretion of the ToolChain. + /// GetTool is overridable, but it's also necessary to avoid inquiring with + /// Driver::ShouldUseClangCompiler() which says "yes" to C, usually. + virtual Tool *SelectTool(const JobAction &JA) const; ---------------- Sorry, my inline comment didn't take the first time. I feel like this comment can be more direct, and avoid the weird "virtualized" wording. How about something like this: /// Choose a tool to handle the action \p JA. /// /// This can be overridden when a particular ToolChain needs to use /// a C compiler other than Clang. I think going into Driver::ShouldUseClangCompiler is more appropriate for the change description than a comment, it isn't really useful documentation to the reader of the interface. http://reviews.llvm.org/D10246 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
