[PATCH] D110730: [SystemZ][z/OS] Introduce initial support for GOFF asm parser

Thu Sep 30 13:24:09 PDT 2021

anirudhp added inline comments.

================
Comment at: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp:128
+
+class SystemZAsmLexerZOS : public SystemZAsmLexerTest {
+protected:
----------------
uweigand wrote:
> Kai wrote:
> > uweigand wrote:
> > > Kai wrote:
> > > > The name should be `SystemZAsmLexerzOS` - as the name of the OS is z/OS.
> > > On the other hand, as the first letter of a component in camel case, shouldn't the "z" still be capitalized?  LexerzOS looks a bit weird to me :-)
> > Agree about the look.
> > IMHO we should make sure that we do not introduce too many variants of the name in the source. E.g. we already have `isOSzOS()` in the `Triple` class.
> > If we continue to use ZOS as the name in all identifiers, then I am ok with that.
> > 
> We also already have Triple::ZOS as well as the ZOS class in clang/lib/Driver/Toolchains/ZOS.cpp and the ZOSTargetInfo class in clang/lib/Basic/Target/OSTargets.h ...
Agree. We can use z/OS in comments wherever needed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110730