[cfe-commits] r163076 - in /cfe/trunk: include/clang/Driver/Util.h lib/Driver/WindowsToolChain.cpp

[Chandler Carruth]

On Sat, Sep 1, 2012 at 6:33 PM, Joao Matos <ripzonetriton at gmail.com> wrote:

> Author: triton
> Date: Sat Sep  1 17:33:43 2012
> New Revision: 163076
>
> URL: http://llvm.org/viewvc/llvm-project?rev=163076&view=rev
> Log:
> Refactored the Windows headers location lookup code. Expose it so
> standalone tools can have access to it.
>

Please revert this. It was not reviewed prior to commit.

I'm strongly opposed to arbitrarily exposing header search logic through a
Util.h header in the driver without some serious thought and planning
around the API. Currently the header search logic is an implementation
detail of the driver. Changing that in any way is a radical departure.



> Modified:
>     cfe/trunk/include/clang/Driver/Util.h
>     cfe/trunk/lib/Driver/WindowsToolChain.cpp
>
> Modified: cfe/trunk/include/clang/Driver/Util.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Util.h?rev=163076&r1=163075&r2=163076&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Driver/Util.h (original)
> +++ cfe/trunk/include/clang/Driver/Util.h Sat Sep  1 17:33:43 2012
> @@ -11,6 +11,8 @@
>  #define CLANG_DRIVER_UTIL_H_
>
>  #include "clang/Basic/LLVM.h"
> +#include <string>
> +#include <vector>
>
>  namespace clang {
>  namespace driver {
> @@ -22,6 +24,9 @@
>    /// ActionList - Type used for lists of actions.
>    typedef SmallVector<Action*, 3> ActionList;
>
> +  /// Gets the default Windows system include directories.
> +  std::vector<std::string> GetWindowsSystemIncludeDirs();
> +
>  } // end namespace driver
>  } // end namespace clang
>
>
> Modified: cfe/trunk/lib/Driver/WindowsToolChain.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/WindowsToolChain.cpp?rev=163076&r1=163075&r2=163076&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Driver/WindowsToolChain.cpp (original)
> +++ cfe/trunk/lib/Driver/WindowsToolChain.cpp Sat Sep  1 17:33:43 2012
> @@ -14,6 +14,7 @@
>  #include "clang/Driver/Compilation.h"
>  #include "clang/Driver/Driver.h"
>  #include "clang/Driver/Options.h"
> +#include "clang/Driver/Util.h"
>  #include "clang/Basic/Version.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/Path.h"
> @@ -304,19 +305,8 @@
>
>  #endif // _MSC_VER
>
> -void Windows::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
> -                                        ArgStringList &CC1Args) const {
> -  if (DriverArgs.hasArg(options::OPT_nostdinc))
> -    return;
> -
> -  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
> -    llvm::sys::Path P(getDriver().ResourceDir);
> -    P.appendComponent("include");
> -    addSystemInclude(DriverArgs, CC1Args, P.str());
> -  }
> -
> -  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
> -    return;
> +std::vector<std::string> clang::driver::GetWindowsSystemIncludeDirs() {
> +  std::vector<std::string> Paths;
>
>  #ifdef _MSC_VER
>    // Honor %INCLUDE%. It should know essential search paths with
> vcvarsall.bat.
> @@ -330,9 +320,9 @@
>        if (d.size() == 0)
>          continue;
>        ++n;
> -      addSystemInclude(DriverArgs, CC1Args, d);
> +      Paths.push_back(d);
>      }
> -    if (n) return;
> +    if (n) return Paths;
>    }
>
>    std::string VSDir;
> @@ -341,25 +331,47 @@
>    // When built with access to the proper Windows APIs, try to actually
> find
>    // the correct include paths first.
>    if (getVisualStudioDir(VSDir)) {
> -    addSystemInclude(DriverArgs, CC1Args, VSDir + "\\VC\\include");
> +    Paths.push_back(VSDir + "\\VC\\include");
>      if (getWindowsSDKDir(WindowsSDKDir))
> -      addSystemInclude(DriverArgs, CC1Args, WindowsSDKDir + "\\include");
> +      Paths.push_back(WindowsSDKDir + "\\include");
>      else
> -      addSystemInclude(DriverArgs, CC1Args,
> -                       VSDir + "\\VC\\PlatformSDK\\Include");
> -    return;
> +      Paths.push_back(VSDir + "\\VC\\PlatformSDK\\Include");
> +    return Paths;
>    }
>  #endif // _MSC_VER
>
>    // As a fallback, select default install paths.
> -  const StringRef Paths[] = {
> +  const StringRef FallbackPaths[] = {
>      "C:/Program Files/Microsoft Visual Studio 10.0/VC/include",
>      "C:/Program Files/Microsoft Visual Studio 9.0/VC/include",
>      "C:/Program Files/Microsoft Visual Studio 9.0/VC/PlatformSDK/Include",
>      "C:/Program Files/Microsoft Visual Studio 8/VC/include",
>      "C:/Program Files/Microsoft Visual Studio 8/VC/PlatformSDK/Include"
>    };
> -  addSystemIncludes(DriverArgs, CC1Args, Paths);
> +
> +  for (unsigned i = 0; i < sizeof(FallbackPaths) /
> sizeof(FallbackPaths[0]); ++i)
> +    Paths.push_back(Paths[i]);
> +
> +  return Paths;
> +}
> +
> +void Windows::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
> +                                        ArgStringList &CC1Args) const {
> +  if (DriverArgs.hasArg(options::OPT_nostdinc))
> +    return;
> +
> +  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
> +    llvm::sys::Path P(getDriver().ResourceDir);
> +    P.appendComponent("include");
> +    addSystemInclude(DriverArgs, CC1Args, P.str());
> +  }
> +
> +  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
> +    return;
> +
> +  std::vector<std::string> Paths = driver::GetWindowsSystemIncludeDirs();
> +  for (size_t i = 0; i < Paths.size(); ++i)
> +    addSystemInclude(DriverArgs, CC1Args, Paths[i]);
>  }
>
>  void Windows::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120904/08f5b1a0/attachment.html>