[PATCH] D154101: [MC] Add three-state parseDirective as a replacement for ParseDirective

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added a comment.

@MaskRay

Thank you for your comments,

In D154101#4461028 <https://reviews.llvm.org/D154101#4461028>, @MaskRay wrote:

> Minor suggestion to the summary: I think you can say that `AsmParser::parseStatement` has the fragile heuristic in the first paragraph, so that curious readers can find this important location immediately.
> Otherwise due to changes to a lot of lib/Target code, they may not notice this quickly.

I tried to improve the summary. If it is what your meant, please feel free to update it (looks like phabricator allows it).

In D154101#4461021 <https://reviews.llvm.org/D154101#4461021>, @MaskRay wrote:

> It looks like `OperandMatchResulTy` could be named more generically to support what `ParseStatus` does in this patch, but unfortunately this is difficult to change as the three enum members have 1000+ references...

I think it could be handled gradually, but I haven't come with a migration plan yet. The most difficult part is tblgenerated methods such as MatchOperandParserImpl, which poison every operand parsing method they call with ParseStatus across all targets.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154101/new/

https://reviews.llvm.org/D154101