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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to