dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. Herald added subscribers: pengfei, fedor.sergeev, kristof.beyls, krytarowski. Herald added a reviewer: JDevlieghere.
I'm not sure if you're still interested in pursuing this, but... In D22943#505523 <https://reviews.llvm.org/D22943#505523>, @echristo wrote: > Seems reasonable to fix the tests? If there are cases where this breaks (correct) tests, I think adding a FIXME is fine, but maybe it can explain in what circumstances `getEffectiveTriple` is currently incorrect. ================ Comment at: lib/Driver/Driver.cpp:2274-2275 if (CCCPrintBindings && !CCGenDiagnostics) { + // FIXME: We should use the effective triple here, but it breaks a test + // on Darwin. llvm::errs() << "# \"" << T->getToolChain().getTripleString() << '"' ---------------- Is the test correct? ================ Comment at: lib/Driver/Tools.cpp:8731 + // inconsistency between the ARM target parser and the expected output. + const llvm::Triple &Triple = getToolChain().getTriple(); + ---------------- This refactoring looks like a nice NFC cleanup you can do regardless. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D22943/new/ https://reviews.llvm.org/D22943 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
