[llvm-commits] [llvm] r89516 - in /llvm/trunk: include/llvm/Target/SubtargetFeature.h lib/Target/SubtargetFeature.cpp tools/lto/LTOCodeGenerator.cpp

[Chris Lattner]

On Nov 20, 2009, at 4:00 PM, Viktor Kutuzov wrote:

> Author: vkutuzov
> Date: Fri Nov 20 18:00:02 2009
> New Revision: 89516
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=89516&view=rev
> Log:
> Added two SubtargetFeatures::AddFeatures methods, which accept a comma-separated string or already parsed command line parameters as input, and some code re-factoring to use these new methods.

Ok, a couple comments below:

> +++ llvm/trunk/include/llvm/Target/SubtargetFeature.h Fri Nov 20 18:00:02 2009
> @@ -22,6 +22,7 @@
> #include <vector>
> #include <cstring>
> #include "llvm/ADT/Triple.h"
> +#include "llvm/Support/CommandLine.h"
> #include "llvm/System/DataTypes.h"

Please drop this #include.

> @@ -93,6 +94,12 @@
>   /// Adding Features.
>   void AddFeature(const std::string &String, bool IsEnabled = true);
> 
> +  /// Add a set of features from the comma-separated string.
> +  void AddFeatures(const std::string &String);

This should take a StringRef instead of std::string.

> +
> +  /// Add a set of features from the parsed command line parameters.
> +  void AddFeatures(const cl::list<std::string> &List);

cl::list inherits from std::vector, so you should be able to pass in a std::vector<std::string> directly.  However, it would be much much better to expose this as taking an array of StringRef's and require the caller to do the unpacking.

-Chris