[PATCH] D25013: [x86][inline-asm][avx512] allow swapping of '{k<num>}' & '{z}' marks

[coby via llvm-commits]

coby added inline comments.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1897-1899
+  if (!getLexer().is(AsmToken::RCurly))
+    return !Error(getLexer().getLoc(), "Expected } at this point");
+  Parser.Lex(); // Eat '}'
----------------
rnk wrote:
> rnk wrote:
> > This can be simplified to:
> >   if (parseToken(AsmToken::RCurly, "expected } at this point"))
> >     return true;
> Any reason not to do this?
Some.
One would be conformity, the second will apply to readability and the other is a somewhat of a flavor issue:
1. While 'parseToken' might (or might not, haven't really checked) be considered as a common way of reporting errors - it is no where to be seen on the currently reviewed source file. hence, at least until global re-factorization is at hand - I would tend to stick to the old and (possibly) good.
2. the title 'parseToken' does brilliant job stating its role as a Parsing subroutine, but fails to reveal its part as an error-reporting mechanism.
3. imho, and as a side note, the entire error reporting handling is a bit of lacky, mainly the need to dictate an error message each time, but it is a subject for another discussion..


Repository:
  rL LLVM

https://reviews.llvm.org/D25013