[Lldb-commits] [PATCH] D58930: Add XCOFF triple object format type for AIX

[Jonas Devlieghere via Phabricator via lldb-commits]

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