[Lldb-commits] [lldb] [llvm] [AArch64][llvm] Tighten SYSP parsing; don't disassemble invalid encodings (PR #182410)

[Jonathan Thackray via lldb-commits]

================
@@ -4265,9 +4219,9 @@ bool AArch64AsmParser::parseSyspAlias(StringRef Name, SMLoc NameLoc,
 
   if (Tok.isNot(AsmToken::Identifier))
     return TokError("expected register identifier");
-  auto Result = tryParseSyspXzrPair(Operands);
-  if (Result.isNoMatch())
-    Result = tryParseGPRSeqPair(Operands);
+  auto Result = tryParseConsecutiveGPRSeqPair</*AllowXZRPair=*/true>(Operands);
+  if (Result.isFailure())
+    return true;
----------------
jthackray wrote:

> What is the difference between Result.isFailure() and Result.isSuccess()

`ParseStatus` is ternary (`NoMatch`, `Success` and `Failure`). `Result.isFailure()` is true only for Failure. But `!Result.isFailure()` is true for both `Success` and `NoMatch`.

> Why we are not testing anymore Result.isNoMatch()?

Previously, the code was:
```
    auto Result = tryParseSyspXzrPair(Operands);
    if (Result.isNoMatch())
      Result = tryParseGPRSeqPair(Operands);
    if (!Result.isSuccess())
      return TokError("specified " + Mnemonic +
                      " op requires a pair of registers");      
```
where `tryParseGPRSeqPair()` only returned Success/Failure, not NoMatch. This then caused `TokError()` to return `true` with a custom message.

The new code has already generated a diagnostic using `return Error(S, FirstRegExpected);` so we just want to return `true` immediately.

Hope this explains the change.

https://github.com/llvm/llvm-project/pull/182410