[PATCH] D97703: [AsmParser][SystemZ][z/OS] Introducing HLASM Comment Syntax

[Scott Linder via Phabricator via llvm-commits]

scott.linder added a comment.

In D97703#2600474 <https://reviews.llvm.org/D97703#2600474>, @anirudhp wrote:

> In D97703#2600429 <https://reviews.llvm.org/D97703#2600429>, @scott.linder wrote:
>
>> Can you add a test? It seems like you already have the `jo *-4` example so just a test that the following does what you expect would be enough for me:
>>
>>   * comment
>>   jo *-4
>
> Thank you for your feedback! I will address the other inline comments shortly. However, for the test, as mentioned in the description, we don't have MCStreamer support yet for the z/OS target, so any test would just fail outright (as mentioned above, we are working on putting it up, but I can't provide a fixed timeframe). I understand that this is problematic, since its best practice to put up a test along with any changes to implementation. Do you have any other recommendations as to how to go about this?

Thank you for the patch! And I'm sorry, I thought I had read the whole description before I hit submit, but evidently not :^)

I don't know the best route, but in similar contexts I have seen the advice in this case (when tests are impossible) to be waiting and including this as part of the first patch for which at least some testing would be possible. I imagine you would like to land this now to simplify the future work, but I would defer to someone with more experience as to whether that is OK in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97703