[PATCH] D90234: [MCParser] Correctly handle Windows line-endings when consuming lexed line comments

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

`AsmCommentConsumer::HandleComment()` is documented as excluding "the newline for single-line comments" which I think is reasonable to interpret as excluding the entire CRLF.  So, the code change LGTM.

The only `AsmCommentConsumer` providers that I see in-tree are in llvm-mca and llvm-exegesis; the latter avoids the problem because it calls `StringRef::trim()` on the provided comment as the first thing it does in its `HandleComment()` override.  It looks like llvm-mca doesn't do that, so the validity of the test is actually dependent on llvm-mca being less deft with its string handling than llvm-exegesis.  That doesn't seem especially robust.

Really what you want here is a unittest that exercises the HandleComment API.  The only existing asm lexer test I see is `llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp` which you might be able to leverage; really you want a target-independent test, though, and the SystemZ test infrastructure does a bunch of things you don't need.

I see @andreadb already approved, so please take this as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90234