hubert.reinterpretcast added inline comments.
================ Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079 + if (log) + log->Printf("sorry: unimplemented for XCOFF"); + return false; ---------------- JDevlieghere wrote: > jasonliu wrote: > > JDevlieghere wrote: > > > 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. > > I think they are different. > > The error message for WASM seems to suggest that it will never ever get > > supported on WASM. > > But it is not the case for XCOFF, we want to indicate that it is not > > implemented yet. > I don't think the error message suggests that at all, and it's definitely not > true. At this point neither XCOFF nor WASM is supported, and that's exactly > what the log message says. > I agree that the error message for WASM does not indicate that the lack of support is inherent or intended to be permanent; however, it is not indicative either of an intent to implement the support. I am not sure what the intent is for WASM, but I do know that the intent for XCOFF is to eventually implement the support. I do not see how using an ambiguous message in this commit (when we know what the intent is) is superior to the alternative of having an unambiguous message. 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