Add Cpp support #362
Add Cpp support #362
Conversation
| @@ -105,6 +105,7 @@ extension Command { | |||
| var args = module.basicArgs | |||
| args += module.optimizationFlags(conf) | |||
| args += ["-L\(prefix)"] | |||
| args += module.languageArgs | |||
There was a problem hiding this comment.
This isn't going to have the desired effect. Technically, if we wanted to do the right thing we would need to look at all dependencies of each ultimate link command, and see if any used C++, and if so then DTRT.
There was a problem hiding this comment.
ah cool, I modified language args to search for any cpp source in dependencies too.
There was a problem hiding this comment.
This still doesn't completely make sense, we want different language specific arguments for when we link versus when we compile. It only makes sense to pass -lc++ when we link.
There was a problem hiding this comment.
I think the name is bad then ... should I rename to lanugageLinkArgs?
(Note that this is linking stage)
There was a problem hiding this comment.
Ah, I missed that. Yeah we should clearly distinguish between linking arguments and compiling arguments.
|
I'm not sure about link error off the top of my head... |
| @@ -29,7 +29,7 @@ public struct Sources { | |||
|
|
|||
| static public var validSwiftExtensions = Set<String>(["swift"]) | |||
|
|
|||
| static public var validCExtensions = Set<String>(["c"]) | |||
| static public var validCExtensions = Set<String>(["c", "m", "cc", "cpp", "cxx"]) | |||
There was a problem hiding this comment.
While we are doing this, is there any reason not to include Objective-C++ at the same time? (.mm)
There was a problem hiding this comment.
I just haven't tested it yet... will do after objc and cpp gets merged
|
Nice work! I left a line note about the linker error. |
ankitspd
force-pushed
the
cpp-support
branch
3 times, most recently
from
f048d3b
to
5ec54ab
Compare
|
@ddunbar Updated this one too... |
| @@ -29,7 +29,7 @@ public struct Sources { | |||
|
|
|||
| static public var validSwiftExtensions = Set<String>(["swift"]) | |||
|
|
|||
| static public var validCExtensions = Set<String>(["c", "m"]) | |||
| static public var validCExtensions = Set<String>(["c", "m", "cc", "cpp", "cxx"]) | |||
There was a problem hiding this comment.
This should include 'mm' for C++.
|
@ddunbar fixed comments |
|
LGTM |
Depends on #360 merge