Details
Diff Detail
- Build Status
Buildable 11413 Build 11413: arc lint + arc unit
Event Timeline
lib/Driver/ToolChain.cpp | ||
---|---|---|
318 | Is this logic intended to replace what was removed from CommonArgs.cpp? Should there be an assert here too? |
lib/Driver/ToolChain.cpp | ||
---|---|---|
318 | Just didn't see the point of that assert. Can you see what the intention might be? I don't see why AddRunTimeLibs() should be callable for any/all triples, do you? I would have had to add llvm::Triple::Unknown to the list of supported OSs, which seemed strange. |
lib/Driver/ToolChain.cpp | ||
---|---|---|
318 | I assume it was just because different OSes have different conventions for the rtlib, and if the OS is unknown and/or there's no particular support, then there might be a bug. But OTOH it doesn't seem bad to have some reasonable default either. For that matter it also seems a little weird that now we are letting the binary format be the determining thing for the Compiler-RT path. But I guess the real issue is that none of the OS, arch, or binary format is really the determining thing; it's really the toolchain/SDK or distribution of the compiler that determines what the path should be, and that can be affected by a lot of other things (e.g. is it the "system" compiler or not, is it a cross compiler, etc). So... yeah I think having this as a default makes as much sense as asserting, and when someone needs to add support for their distribution, then I suppose it's on them to refactor as needed and keep the tests working. |
Is this logic intended to replace what was removed from CommonArgs.cpp? Should there be an assert here too?