[PATCH] D153204: RISCVAsmParser: support comments in more places

Sergei Barannikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 21 01:12:52 PDT 2023


barannikov88 added a comment.

@MaskRay
I don't doubt your experience, nor your will to help others, and I certainly didn't mean to offend. My point was that creating and insisting on an alternative review does not encourage newcomers to contribute.
I think it would be better if we helped Abel to get his review right instead. It would also save everyone's time arguing about which review is better and relieve the reviewers from having to judge.

@abel-bernabeu

>> The change proposed by Sergei Barannikov was to just search and replace getLexer().Lex() for getParser.Lex() or simply Lex(), which is exactly what I did on the original merge request. This solution was preferred over surgically fixing just the occurrences that were deemed needed for most common cases.
>
> I think this was a suggestion to help identify the problem, but not that the proposed patch should do this.

It was a suggestion to help identify the problem, I don't expect any credits for this.

> Also, regarding the verboseness of the patch. I think this patch strikes a good balance, the other one has too many paragraphs.

I also agree here. The description could be much less verbose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153204



More information about the llvm-commits mailing list