JDevlieghere requested changes to this revision. JDevlieghere added inline comments. This revision now requires changes to proceed.
================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:1470 + case Triple::XCOFF: + // TODO: Falling through for XCOFF format for now. + break; ---------------- This is confusing, you say fall through but you break? I would prefer a `llvm_unreachable("XCOFF not yet implemented");` here and elsewhere in this patch. ================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:1486 + case Triple::XCOFF: + // TODO: Falling through for XCOFF format for now. + break; ---------------- See previous comment. ================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4410 + case llvm::Triple::XCOFF: + llvm_unreachable("to be determined for XCOFF format"); case llvm::Triple::COFF: ---------------- See previous comment. ================ Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079 + if (log) + log->Printf("sorry: unimplemented for XCOFF"); + return false; ---------------- jasonliu wrote: > apaprocki wrote: > > No need to be `sorry:` :) This should probably just say `error: XCOFF is > > unimplemented` to be more direct in case anything is expecting "error:" in > > the output. > Sure. Will address in next revision. Just bundle this with the WASM case, the error message is correct for both. ================ Comment at: llvm/lib/MC/MCContext.cpp:165 + case MCObjectFileInfo::IsXCOFF: + // TODO: Need to implement class MCSymbolXCOFF. + break; ---------------- See previous comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58930/new/ https://reviews.llvm.org/D58930 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits