[llvm-commits] [PATCH] lld driver

[Michael Spencer]

================
Comment at: include/lld/Driver/Driver.h:40
@@ +39,3 @@
+public:
+  enum class Flavor {
+    invalid,
----------------
Alex Rosenberg wrote:
> Having an enum means adding another linker has to happen in a bunch of spots. Can this be done instead with a registry of templates of some kind?
Yes, but I would like to do that as a separate patch.

================
Comment at: tools/lld/lld.cpp:96
@@ +95,3 @@
+  // On mac treat ld as ld64.
+#ifdef __APPLE__
+  if (flavor == Driver::Flavor::ld)
----------------
Alex Rosenberg wrote:
> This presumes that the output is for Darwin. My use case is as a cross-compiler where ld does not mean ld64 and I'd really like to be able to use that on OS X.
> 
> Can we determine this instead based on the target OS?
Ah, I see. I had assumed ld was always ld64 on mac.

I don't really like always assuming ld is ld64 when targeting mac either.

As most cross tools use prefixed naming (arm-linux-ld) I think that when invoked hosted on mac as just ld, it should always act as ld64. If it has any prefixes, it should act as ld.

Does anyone know of any cases with a prefixed ld that runs as ld64?

Other options are basing it purely on sys::getDefaultTarget, which is a configure time setting. Or just adding a config option specifically for this.

================
Comment at: include/lld/Core/ErrorOr.h:10-12
@@ +9,5 @@
+///
+/// \file
+///
+/// Provides ErrorOr<T> smart pointer.
+///
----------------
Sean Silva wrote:
> Michael Spencer wrote:
> > Sean Silva wrote:
> > > Since this class seems generally useful, it is probably worth mentioning here why this file is not in libSupport.
> > It's c++11. Although it's still useful for reference and value types without c++11.
> Is there any current usage in this code that would not be possible if this were located in libSupport? I think it would be really beneficial to have this more generally available. "reference and value types" seems like a pretty large class of objects that this could be useful for.
> 
> Also, I'm not sure if this was understood from what I said, but I think that the reason should be explicitly mentioned in this header comment (if it isn't integrated into libSupport, that is).
I'll add a comment and look at moving it to support in a separate patch.


http://llvm-reviews.chandlerc.com/D169