[PATCH] D152273: [RISCV] Don't persist invalid feature state on .option arch error

Tue Jun 6 08:45:07 PDT 2023

MaskRay added inline comments.

================
Comment at: llvm/test/MC/RISCV/option-invalid.s:50

+## Make sure the above error isn't sticky
+# CHECK-NOT: :[[#@LINE+1]]:
----------------
jrtc27 wrote:
> MaskRay wrote:
> > jrtc27 wrote:
> > > MaskRay wrote:
> > > > 
> > > Short comments don't normally get full stops?
> > In comments, a complete sentence gets a full stop. An incomplete one doesn't.
> > There may be cases where a short complete sentence doesn't get a full stop, but I think we don't encourage that:)
> > 
> > (
> > As a difference, diagnostic messages typically do not get full stops
> > https://llvm.org/docs/CodingStandards.html#error-and-warning-messages
> > )
> Like many comments I view this more as a sentence fragment that happens to technically be a complete sentence. Adding a full stop changes the tone of the comment in a way that isn't appropriate IMO. I'm not fundamentally opposed to it, it just feels quite unnatural to do.
OK, I don't actually mind much about the comment style in the tests. It was just a suggestion. Feel free to land the patch as is:)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152273