[PATCH] Make ToolChain::SelectTool virtual.

Chandler Carruth chandlerc at gmail.com
Tue Jun 9 01:22:40 PDT 2015


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/






More information about the cfe-commits mailing list