[MC][WebAssembly] Allow accurate errors in doBeforeLabelEmit

[sbc100]

Summary

Although we only currently have one error produced in this function I am
working on changes right now that add some more. This change makes the
error location more accurate.

[dschuff]

Don't forget to make a build of LLVM with all the targets to make sure you get everything.

llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

737

Isn't there an implementation for this function that needs to be updated?

[sbc100]

• fix

[sbc100]

• feedback

llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

737

Oops, done.

llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h

public:
 
   virtual const MCExpr *applyModifierToExpr(const MCExpr *E,
                                             MCSymbolRefExpr::VariantKind,
                                             MCContext &Ctx) {
     return nullptr;
   }
 
   // For actions that have to be performed before a label is emitted
-  virtual void doBeforeLabelEmit(MCSymbol *Symbol) {}
+  virtual void doBeforeLabelEmit(MCSymbol *Symbol, SMLoc IDLoc) {}
   
   virtual void onLabelParsed(MCSymbol *Symbol) {}
 
   /// Ensure that all previously parsed instructions have been emitted to the
   /// output streamer, if the target does not emit them immediately.
   virtual void flushPendingInstructions(MCStreamer &Out) {}
 
   virtual const MCExpr *createTargetUnaryExpr(const MCExpr *E,

llvm/lib/MC/MCParser/AsmParser.cpp

case AsmToken::Colon: {
     // addBlankLine calls().
     if (getTok().is(AsmToken::EndOfStatement)) {
       Lex();
     }
 
     if (discardLTOSymbol(IDVal))
       return false;
 
-    getTargetParser().doBeforeLabelEmit(Sym);
+    getTargetParser().doBeforeLabelEmit(Sym, IDLoc);
 
     // Emit the label.
     if (!getTargetParser().isParsingMSInlineAsm())
       Out.emitLabel(Sym, IDLoc);
 
     // If we are generating dwarf for assembly source files then gather the
     // info to make a dwarf label entry for this label if needed.
     if (enabledGenDwarfForAssembly())
if (getTok().is(AsmToken::EndOfStatement))
     return Error(LabelLoc,
                  "Cannot have just a label for an HLASM inline asm statement");
 
   MCSymbol *Sym = getContext().getOrCreateSymbol(
       getContext().getAsmInfo()->shouldEmitLabelsInUpperCase()
           ? LabelVal.upper()
           : LabelVal);
 
-  getTargetParser().doBeforeLabelEmit(Sym);
+  getTargetParser().doBeforeLabelEmit(Sym, LabelLoc);
 
   // Emit the label.
   Out.emitLabel(Sym, LabelLoc);
 
   // If we are generating dwarf for assembly source files then gather the
   // info to make a dwarf label entry for this label if needed.
   if (enabledGenDwarfForAssembly())
     MCGenDwarfLabelEntry::Make(Sym, &getStreamer(), getSourceManager(),

llvm/lib/MC/MCParser/MasmParser.cpp

case AsmToken::Colon: {
     }
 
     // Consume any end of statement token, if present, to avoid spurious
     // addBlankLine calls().
     if (getTok().is(AsmToken::EndOfStatement)) {
       Lex();
     }
 
-    getTargetParser().doBeforeLabelEmit(Sym);
+    getTargetParser().doBeforeLabelEmit(Sym, IDLoc);
 
     // Emit the label.
     if (!getTargetParser().isParsingMSInlineAsm())
       Out.emitLabel(Sym, IDLoc);
 
     // If we are generating dwarf for assembly source files then gather the
     // info to make a dwarf label entry for this label if needed.
     if (enabledGenDwarfForAssembly())

llvm/lib/MC/MCParser/WasmAsmParser.cpp

bool parseSectionDirective(StringRef, SMLoc loc) {
     }
 
     getStreamer().switchSection(WS);
     return false;
   }
 
   // TODO: This function is almost the same as ELFAsmParser::ParseDirectiveSize
   // so maybe could be shared somehow.
-  bool parseDirectiveSize(StringRef, SMLoc) {
+  bool parseDirectiveSize(StringRef, SMLoc Loc) {
     StringRef Name;
     if (Parser->parseIdentifier(Name))
       return TokError("expected identifier in directive");
     auto Sym = getContext().getOrCreateSymbol(Name);
     if (expect(AsmToken::Comma, ","))
       return true;
     const MCExpr *Expr;
     if (Parser->parseExpression(Expr))

llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

#include "ARMGenAsmMatcher.inc"
   const char *getCustomOperandDiag(ARMMatchResultTy MatchError);
 
   void FilterNearMisses(SmallVectorImpl<NearMissInfo> &NearMissesIn,
                         SmallVectorImpl<NearMissMessage> &NearMissesOut,
                         SMLoc IDLoc, OperandVector &Operands);
   void ReportNearMisses(SmallVectorImpl<NearMissInfo> &NearMisses, SMLoc IDLoc,
                         OperandVector &Operands);
 
-  void doBeforeLabelEmit(MCSymbol *Symbol) override;
+  void doBeforeLabelEmit(MCSymbol *Symbol, SMLoc IDLoc) override;
 
   void onLabelParsed(MCSymbol *Symbol) override;
 };
 
 /// ARMOperand - Instances of this class represent a parsed ARM machine
 /// operand.
 class ARMOperand : public MCParsedAsmOperand {
   enum KindTy {
if (parseEOL() || check(!hasARM(), L, "target does not support ARM mode"))
     return true;
 
   if (isThumb())
     SwitchMode();
   getParser().getStreamer().emitAssemblerFlag(MCAF_Code32);
   return false;
 }
 
-void ARMAsmParser::doBeforeLabelEmit(MCSymbol *Symbol) {
+void ARMAsmParser::doBeforeLabelEmit(MCSymbol *Symbol, SMLoc IDLoc) {
   // We need to flush the current implicit IT block on a label, because it is
   // not legal to branch into an IT block.
   flushPendingInstructions(getStreamer());
 }
 
 void ARMAsmParser::onLabelParsed(MCSymbol *Symbol) {
   if (NextSymbolIsThumb) {
     getParser().getStreamer().emitThumbFunc(Symbol);

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

case Match_InvalidOperand: {
           ErrorLoc = IDLoc;
       }
       return Parser.Error(ErrorLoc, "invalid operand for instruction");
     }
     }
     llvm_unreachable("Implement any new match types added!");
   }
 
-  void doBeforeLabelEmit(MCSymbol *Symbol) override {
+  void doBeforeLabelEmit(MCSymbol *Symbol, SMLoc IDLoc) override {
     // Code below only applies to labels in text sections.
     auto CWS = cast<MCSectionWasm>(getStreamer().getCurrentSection().first);
     if (!CWS || !CWS->getKind().isText())
       return;
 
     auto WasmSym = cast<MCSymbolWasm>(Symbol);
     // Unlike other targets, we don't allow data in text sections (labels
     // declared with .type @object).
     if (WasmSym->getType() == wasm::WASM_SYMBOL_TYPE_DATA) {
-      Parser.Error(Parser.getTok().getLoc(),
+      Parser.Error(IDLoc,
                    "Wasm doesn\'t support data symbols in text sections");
       return;
     }
 
     // Start a new section for the next function automatically, since our
     // object writer expects each function to have its own section. This way
     // The user can't forget this "convention".
     auto SymName = Symbol->getName();

llvm/test/MC/WebAssembly/basic-assembly-errors.s

-# RUN: not llvm-mc -triple=wasm32-unknown-unknown -mattr=+simd128,+nontrapping-fptoint,+exception-handling < %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple=wasm32-unknown-unknown -mattr=+simd128,+nontrapping-fptoint,+exception-handling %s 2>&1 | FileCheck %s
 
 # CHECK: invalid operand for instruction
 # (must be 0.0 or similar)
     f32.const 0
 
-# CHECK: Wasm doesn't support data symbols in text sections
-  .type objerr,@object
+# CHECK: basic-assembly-errors.s:9:1: error: Wasm doesn't support data symbols in text sections
+.type objerr,@object
 objerr:
 
 # CHECK: End of block construct with no start: end_try
     end_try
 test0:
     .functype   test0 () -> ()
 # CHECK: Block construct type mismatch, expected: end_function, instead got: end_loop
     end_loop