[PATCH] MinGW toolchain

Martell Malone martellmalone at gmail.com
Sat Apr 19 11:59:53 PDT 2014


  Okay so I have looked at all the comments and I'm trying to get all the pieces together to get this working best as possible.

  @rubenvb I first would like to thank you for allowing this patch to be included, :)

  With you being the original author I'd like to poke your brain about a thing or two.
  We could have two c++ libraries (libstdc++ or libc++)
  Based on which on is going to be used we will have todo the includes accordingly.
  Can I get your thoughts on this, should I implement this in the driver or should we always assume libstdc++?

  @yaron.kernen I have made a few inline comments with reference to the different questions posed.


================
Comment at: lib/Frontend/InitHeaderSearch.cpp:415-423
@@ -414,11 +371,1 @@
-      // mingw.org C++ include paths
-#if defined(LLVM_ON_WIN32)
-      AddMinGWCPlusPlusIncludePaths("c:/MinGW/lib/gcc", "mingw32", "4.7.0");
-      AddMinGWCPlusPlusIncludePaths("c:/MinGW/lib/gcc", "mingw32", "4.7.1");
-      AddMinGWCPlusPlusIncludePaths("c:/MinGW/lib/gcc", "mingw32", "4.7.2");
-      AddMinGWCPlusPlusIncludePaths("c:/MinGW/lib/gcc", "mingw32", "4.7.3");
-      AddMinGWCPlusPlusIncludePaths("c:/MinGW/lib/gcc", "mingw32", "4.8.0");
-      AddMinGWCPlusPlusIncludePaths("c:/MinGW/lib/gcc", "mingw32", "4.8.1");
-      AddMinGWCPlusPlusIncludePaths("c:/MinGW/lib/gcc", "mingw32", "4.8.2");
-#endif
       break;
----------------
As @rubenvb said I think we should really look at mingw.org as legacy.
The simplest way to have support for it would be to leave these lines in the code intact.

That way we could focus on a mingw64 driver while still keeping mingw.org's currently supported state.

I can do another PR at a later stage to add mingw.org support to the driver.
After we have solved some other issues with it.
I.E I think it would be best to split the work load here.

================
Comment at: lib/Frontend/InitHeaderSearch.cpp:307-308
@@ -342,3 +306,4 @@
 
-  if ( os != llvm::Triple::RTEMS )
+  if ((os != llvm::Triple::RTEMS) &&
+     (!((os == llvm::Triple::Win32) && (triple.getEnvironment() == llvm::Triple::GNU))))
     AddPath("/usr/include", ExternCSystem, false);
----------------
This isn't exactly the best looking code.
It gets the job done but I would prefer to have a better implementation of disabling that include.
Suggestions would be much appreciated 

================
Comment at: lib/Driver/MinGWToolChain.cpp:33
@@ +32,3 @@
+  : ToolChain(D, Triple, Args) {
+  //TODO: libc++ directory
+
----------------
David Majnemer wrote:
> This todo is pretty vague, can you add more context?
@majnemer this comment comes from the original patch from @rubenvb
I will add a space and clarify exactly what is meant here.

As @yaron.keren has said we should be finding libstdc++ from the gcc location.

================
Comment at: lib/Driver/MinGWToolChain.cpp:48
@@ +47,3 @@
+      GCCVersion = llvm::sys::path::filename(entry->path());
+    //TODO report error: "no MinGW-w64 GCC installation found"
+  }
----------------
David Majnemer wrote:
> Need a space before the start of the comment.
Will fix this in the updated diff. Thank you


http://reviews.llvm.org/D3420






More information about the cfe-commits mailing list