[MC] Make MCStreamer aware of AsmParser's StartTokLoc

A SMLoc allows MCStreamer to report location-aware diagnostics, which
were previously done by adding SMLoc to various methods (e.g. emit*) in an ad-hoc way.

Since the file:line is most important, the column is less important and
the start token location suffices in many cases, this patch reverts
b7e7131af2dd7bdb03fa42a3bc1b4bc72ab95ce1

```
// old
symbol-binding-changed.s:6:8: error: local changed binding to STB_GLOBAL
.globl local
       ^
// new
symbol-binding-changed.s:6:1: error: local changed binding to STB_GLOBAL
.globl local
^
```

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D90511

include/llvm/MC/MCELFStreamer.h

@@ -46,8 +46,7 @@ public:
   void emitAssemblerFlag(MCAssemblerFlag Flag) override;
   void emitThumbFunc(MCSymbol *Func) override;
   void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
-                           SMLoc Loc = SMLoc()) override;
   void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;

include/llvm/MC/MCStreamer.h

@@ -215,6 +215,10 @@ class MCStreamer {
   /// PushSection.
   SmallVector<std::pair<MCSectionSubPair, MCSectionSubPair>, 4> SectionStack;
 
+  /// Pointer to the parser's SMLoc if available. This is used to provide
+  /// locations for diagnostics.
+  const SMLoc *StartTokLocPtr = nullptr;
+
   /// The next unique ID to use when creating a WinCFI-related section (.pdata
   /// or .xdata). This ID ensures that we have a one-to-one mapping from
   /// code section to unwind info section, which MSVC's incremental linker
@@ -259,6 +263,11 @@ public:
     TargetStreamer.reset(TS);
   }
 
+  void setStartTokLocPtr(const SMLoc *Loc) { StartTokLocPtr = Loc; }
+  SMLoc getStartTokLoc() const {
+    return StartTokLocPtr ? *StartTokLocPtr : SMLoc();
+  }
+
   /// State management
   ///
   virtual void reset();
@@ -508,8 +517,8 @@ public:
   virtual void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol);
 
   /// Add the given \p Attribute to \p Symbol.
-  virtual bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  virtual bool emitSymbolAttribute(MCSymbol *Symbol,
-                                   SMLoc Loc = SMLoc()) = 0;
+                                   MCSymbolAttr Attribute) = 0;
 
   /// Set the \p DescValue for the \p Symbol.
   ///

include/llvm/MC/MCWasmStreamer.h

@@ -44,8 +44,7 @@ public:
   void emitAssemblerFlag(MCAssemblerFlag Flag) override;
   void emitThumbFunc(MCSymbol *Func) override;
   void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
-                           SMLoc) override;
   void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;

include/llvm/MC/MCWinCOFFStreamer.h

@@ -43,8 +43,7 @@ public:
   void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
   void emitAssemblerFlag(MCAssemblerFlag Flag) override;
   void emitThumbFunc(MCSymbol *Func) override;
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
-                           SMLoc Loc = SMLoc()) override;
   void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override;
   void BeginCOFFSymbolDef(MCSymbol const *Symbol) override;
   void EmitCOFFSymbolStorageClass(int StorageClass) override;

include/llvm/MC/MCXCOFFStreamer.h

@@ -19,8 +19,7 @@ public:
                   std::unique_ptr<MCObjectWriter> OW,
                   std::unique_ptr<MCCodeEmitter> Emitter);
 
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
-                           SMLoc Loc = SMLoc()) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
   void emitZerofill(MCSection *Section, MCSymbol *Symbol = nullptr,

lib/MC/MCAsmStreamer.cpp

@@ -157,8 +157,7 @@ public:
 
   void emitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
   void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
-                           SMLoc) override;
 
   void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override;
   void BeginCOFFSymbolDef(const MCSymbol *Symbol) override;
@@ -636,7 +635,7 @@ void MCAsmStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {
 }
 
 bool MCAsmStreamer::emitSymbolAttribute(MCSymbol *Symbol,
-                                        MCSymbolAttr Attribute, SMLoc) {
+                                        MCSymbolAttr Attribute) {
   switch (Attribute) {
   case MCSA_Invalid: llvm_unreachable("Invalid symbol attribute");
   case MCSA_ELF_TypeFunction:    /// .type _foo, STT_FUNC  # aka @function

lib/MC/MCELFStreamer.cpp

@@ -187,8 +187,7 @@ static unsigned CombineSymbolTypes(unsigned T1, unsigned T2) {
   return T2;
 }
 
-bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute,
+bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) {
-                                        SMLoc Loc) {
   auto *Symbol = cast<MCSymbolELF>(S);
 
   // Adding a symbol attribute always introduces the symbol, note that an
@@ -230,8 +229,9 @@ bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute,
     // traditionally set the binding to STB_GLOBAL. This is error-prone, so we
     // error on such cases. Note, we also disallow changed binding from .local.
     if (Symbol->isBindingSet() && Symbol->getBinding() != ELF::STB_GLOBAL)
-      getContext().reportError(Loc, Symbol->getName() +
+      getContext().reportError(getStartTokLoc(),
-                                        " changed binding to STB_GLOBAL");
+                               Symbol->getName() +
+                                   " changed binding to STB_GLOBAL");
     Symbol->setBinding(ELF::STB_GLOBAL);
     Symbol->setExternal(true);
     break;
@@ -241,16 +241,17 @@ bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute,
     // For `.global x; .weak x`, both MC and GNU as set the binding to STB_WEAK.
     // We emit a warning for now but may switch to an error in the future.
     if (Symbol->isBindingSet() && Symbol->getBinding() != ELF::STB_WEAK)
-      getContext().reportWarning(Loc, Symbol->getName() +
+      getContext().reportWarning(
-                                          " changed binding to STB_WEAK");
+          getStartTokLoc(), Symbol->getName() + " changed binding to STB_WEAK");
     Symbol->setBinding(ELF::STB_WEAK);
     Symbol->setExternal(true);
     break;
 
   case MCSA_Local:
     if (Symbol->isBindingSet() && Symbol->getBinding() != ELF::STB_LOCAL)
-      getContext().reportError(Loc, Symbol->getName() +
+      getContext().reportError(getStartTokLoc(),
-                                        " changed binding to STB_LOCAL");
+                               Symbol->getName() +
+                                   " changed binding to STB_LOCAL");
     Symbol->setBinding(ELF::STB_LOCAL);
     Symbol->setExternal(false);
     break;

lib/MC/MCMachOStreamer.cpp

@@ -93,8 +93,7 @@ public:
   void emitBuildVersion(unsigned Platform, unsigned Major, unsigned Minor,
                         unsigned Update, VersionTuple SDKVersion) override;
   void emitThumbFunc(MCSymbol *Func) override;
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
-                           SMLoc Loc = SMLoc()) override;
   void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
@@ -291,8 +290,8 @@ void MCMachOStreamer::emitThumbFunc(MCSymbol *Symbol) {
   cast<MCSymbolMachO>(Symbol)->setThumbFunc();
 }
 
-bool MCMachOStreamer::emitSymbolAttribute(MCSymbol *Sym, MCSymbolAttr Attribute,
+bool MCMachOStreamer::emitSymbolAttribute(MCSymbol *Sym,
-                                          SMLoc) {
+                                          MCSymbolAttr Attribute) {
   MCSymbolMachO *Symbol = cast<MCSymbolMachO>(Sym);
 
   // Indirect symbols are handled differently, to match how 'as' handles

lib/MC/MCNullStreamer.cpp

@@ -25,7 +25,8 @@ namespace {
     bool hasRawTextSupport() const override { return true; }
     void emitRawTextImpl(StringRef String) override {}
 
-    bool emitSymbolAttribute(MCSymbol *, MCSymbolAttr, SMLoc) override {
+    bool emitSymbolAttribute(MCSymbol *Symbol,
+                             MCSymbolAttr Attribute) override {
       return true;
     }
 

lib/MC/MCParser/AsmParser.cpp

@@ -126,6 +126,7 @@ private:
   SourceMgr::DiagHandlerTy SavedDiagHandler;
   void *SavedDiagContext;
   std::unique_ptr<MCAsmParserExtension> PlatformParser;
+  SMLoc StartTokLoc;
 
   /// This is the current buffer index we're lexing from as managed by the
   /// SourceMgr object.
@@ -709,6 +710,8 @@ AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
   // Set our own handler which calls the saved handler.
   SrcMgr.setDiagHandler(DiagHandler, this);
   Lexer.setBuffer(SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer());
+  // Make MCStreamer aware of the StartTokLoc for locations in diagnostics.
+  Out.setStartTokLocPtr(&StartTokLoc);
 
   // Initialize the platform / file format parser.
   switch (Ctx.getObjectFileInfo()->getObjectFileType()) {
@@ -742,6 +745,8 @@ AsmParser::~AsmParser() {
   assert((HadError || ActiveMacros.empty()) &&
          "Unexpected active macro instantiation!");
 
+  // Remove MCStreamer's reference to the parser SMLoc.
+  Out.setStartTokLocPtr(nullptr);
   // Restore the saved diagnostics handler and context for use during
   // finalization.
   SrcMgr.setDiagHandler(SavedDiagHandler, SavedDiagContext);
@@ -1705,6 +1710,7 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info,
   SMLoc IDLoc = ID.getLoc();
   StringRef IDVal;
   int64_t LocalLabelVal = -1;
+  StartTokLoc = ID.getLoc();
   if (Lexer.is(AsmToken::HashDirective))
     return parseCppHashLineFilenameComment(IDLoc,
                                            !isInsideMacroInstantiation());
@@ -4881,7 +4887,7 @@ bool AsmParser::parseDirectiveSymbolAttribute(MCSymbolAttr Attr) {
     if (Sym->isTemporary())
       return Error(Loc, "non-local symbol required");
 
-    if (!getStreamer().emitSymbolAttribute(Sym, Attr, Loc))
+    if (!getStreamer().emitSymbolAttribute(Sym, Attr))
       return Error(Loc, "unable to emit symbol attribute");
     return false;
   };

lib/MC/MCParser/ELFAsmParser.cpp

@@ -178,13 +178,13 @@ bool ELFAsmParser::ParseDirectiveSymbolAttribute(StringRef Directive, SMLoc) {
   if (getLexer().isNot(AsmToken::EndOfStatement)) {
     while (true) {
       StringRef Name;
-      SMLoc Loc = getTok().getLoc();
+
       if (getParser().parseIdentifier(Name))
         return TokError("expected identifier in directive");
 
       MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
 
-      getStreamer().emitSymbolAttribute(Sym, Attr, Loc);
+      getStreamer().emitSymbolAttribute(Sym, Attr);
 
       if (getLexer().is(AsmToken::EndOfStatement))
         break;

lib/MC/MCWasmStreamer.cpp

@@ -77,8 +77,7 @@ void MCWasmStreamer::emitWeakReference(MCSymbol *Alias,
   Alias->setVariableValue(Value);
 }
 
-bool MCWasmStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute,
+bool MCWasmStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) {
-                                         SMLoc) {
   assert(Attribute != MCSA_IndirectSymbol && "indirect symbols not supported");
 
   auto *Symbol = cast<MCSymbolWasm>(S);

lib/MC/MCWinCOFFStreamer.cpp

@@ -107,8 +107,8 @@ void MCWinCOFFStreamer::emitThumbFunc(MCSymbol *Func) {
   llvm_unreachable("not implemented");
 }
 
-bool MCWinCOFFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute,
+bool MCWinCOFFStreamer::emitSymbolAttribute(MCSymbol *S,
-                                            SMLoc) {
+                                            MCSymbolAttr Attribute) {
   auto *Symbol = cast<MCSymbolCOFF>(S);
   getAssembler().registerSymbol(*Symbol);
 

lib/MC/MCXCOFFStreamer.cpp

@@ -29,8 +29,8 @@ MCXCOFFStreamer::MCXCOFFStreamer(MCContext &Context,
     : MCObjectStreamer(Context, std::move(MAB), std::move(OW),
                        std::move(Emitter)) {}
 
-bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym, MCSymbolAttr Attribute,
+bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym,
-                                          SMLoc) {
+                                          MCSymbolAttr Attribute) {
   auto *Symbol = cast<MCSymbolXCOFF>(Sym);
   getAssembler().registerSymbol(*Symbol);
 

lib/Object/RecordStreamer.cpp

@@ -97,7 +97,7 @@ void RecordStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {
 }
 
 bool RecordStreamer::emitSymbolAttribute(MCSymbol *Symbol,
-                                         MCSymbolAttr Attribute, SMLoc) {
+                                         MCSymbolAttr Attribute) {
   if (Attribute == MCSA_Global || Attribute == MCSA_Weak)
     markGlobal(*Symbol, Attribute);
   if (Attribute == MCSA_LazyReference)
@@ -226,7 +226,7 @@ void RecordStreamer::flushSymverDirectives() {
       // Don't use EmitAssignment override as it always marks alias as defined.
       MCStreamer::emitAssignment(Alias, Value);
       if (Attr != MCSA_Invalid)
-        emitSymbolAttribute(Alias, Attr, SMLoc());
+        emitSymbolAttribute(Alias, Attr);
     }
   }
 }

lib/Object/RecordStreamer.h

@@ -48,8 +48,7 @@ public:
   void emitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override;
   void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
   void emitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
-                           SMLoc Loc) override;
   void emitZerofill(MCSection *Section, MCSymbol *Symbol, uint64_t Size,
                     unsigned ByteAlignment, SMLoc Loc = SMLoc()) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,

test/MC/ELF/symbol-binding-changed.s

@@ -1,17 +1,17 @@
 # RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
 
-# CHECK: {{.*}}.s:[[#@LINE+3]]:8: error: local changed binding to STB_GLOBAL
+# CHECK: {{.*}}.s:[[#@LINE+3]]:1: error: local changed binding to STB_GLOBAL
 local:
 .local local
 .globl local
 
 ## `.globl x; .weak x` matches the GNU as behavior. We issue a warning for now.
-# CHECK: {{.*}}.s:[[#@LINE+3]]:7: warning: global changed binding to STB_WEAK
+# CHECK: {{.*}}.s:[[#@LINE+3]]:1: warning: global changed binding to STB_WEAK
 global:
 .global global
 .weak global
 
-# CHECK: {{.*}}.s:[[#@LINE+3]]:8: error: weak changed binding to STB_LOCAL
+# CHECK: {{.*}}.s:[[#@LINE+3]]:1: error: weak changed binding to STB_LOCAL
 weak:
 .weak weak
 .local weak

tools/llvm-exegesis/lib/SnippetFile.cpp

@@ -89,8 +89,7 @@ private:
   // We only care about instructions, we don't implement this part of the API.
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override {}
-  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute,
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override {
-                           SMLoc) override {
     return false;
   }
   void emitValueToAlignment(unsigned ByteAlignment, int64_t Value,

tools/llvm-mca/CodeRegionGenerator.cpp

@@ -52,7 +52,7 @@ public:
     Regions.addInstruction(Inst);
   }
 
-  bool emitSymbolAttribute(MCSymbol *, MCSymbolAttr, SMLoc) override {
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override {
     return true;
   }
 

unittests/CodeGen/TestAsmPrinter.h

@@ -28,8 +28,8 @@ public:
 
   // These methods are pure virtual in MCStreamer, thus, have to be overridden:
 
-  MOCK_METHOD3(emitSymbolAttribute,
+  MOCK_METHOD2(emitSymbolAttribute,
-               bool(MCSymbol *Symbol, MCSymbolAttr Attribute, SMLoc Loc));
+               bool(MCSymbol *Symbol, MCSymbolAttr Attribute));
   MOCK_METHOD3(emitCommonSymbol,
                void(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment));
   MOCK_METHOD5(emitZerofill,