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

Thu Jun 29 13:20:34 PDT 2023

MaskRay added a comment.

In D154101#4461449 <https://reviews.llvm.org/D154101#4461449>, @barannikov88 wrote:

> @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).

LG. Thanks!

> 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.

I haven't investigated closely but I guess some user-defined conversion function may help gradual migration.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154101