[PATCH] D24047: [MC] Defer asm errors to post-statement failure

[Reid Kleckner via llvm-commits]

rnk added a comment.

Can you provide some motivating examples for how this improves diagnostic quality by eliminating duplicates? Clang's diagnostics aren't buffered, and our recovery is still acceptable.


================
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:72
@@ -69,1 +71,3 @@
 
+  typedef struct {
+    SMLoc Loc;
----------------
Just `struct MCPendingError` would be more C++-y

================
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:74
@@ +73,3 @@
+    SMLoc Loc;
+    SmallString<256> Msg;
+    ArrayRef<SMRange> Ranges;
----------------
Right now sizeof(Msg) is 256+3*sizeof(void*), and you're putting MCPendingError in a vector. You should drop this down to 64 or use std::string.

================
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:75
@@ +74,3 @@
+    SmallString<256> Msg;
+    ArrayRef<SMRange> Ranges;
+  } MCPendingError;
----------------
This ArrayRef seems like a good way to create dangling references to local ranges arrays. Does any code actually use this functionality? Maybe we can delete it?

================
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:91
@@ +90,3 @@
+  /// Flag tracking whether any errors have been encountered.
+  unsigned HadError : 1;
+
----------------
Maybe move ShowParsedOperands down here if you want to be in the same bitfield, or just make this bool.


https://reviews.llvm.org/D24047