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

Sean Fertile via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Mar 8 07:46:42 PST 2019

sfertile accepted this revision.
sfertile added a comment.


Comment at: llvm/lib/MC/MCContext.cpp:165
+    case MCObjectFileInfo::IsXCOFF:
+      // TODO: Need to implement class MCSymbolXCOFF.
+      break;
jasonliu wrote:
> sfertile wrote:
> > jasonliu wrote:
> > > JDevlieghere wrote:
> > > > See previous comment.
> > > It is certain that we will need MCSymbolXCOFF. But before we run into cases where we actually need a MCSymbolXCOFF, we could use the generic MCSymbol first for XCOFF platform. So I don't want to put a llvm_unreachable here. 
> > Would it make sense to add an llvm_unreachable now, and the first patch that actually uses an MCSymbol stubs out the class and removes the unreachable?
> The first patch that uses MCSymbol do not necessarily need to stub out MCSymbolXCOFF, as MCSymbol seems to be generic and usable until we are doing some XCOFF specific things that needs to be represented by MCSymbolXCOFF. If the intention is MCSymbol should never get used, and different object file should have their own MCSymbolXXX class from the start, then I could add in the llvm_unreachable here, and I would also propose to replace the "return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name, IsTemporary);" with an llvm_unreachable as well. 
Ok I think falling through to create the generic MCSymbol in this patch is reasonable. 

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list