michaelplatings added a comment. The change seems OK to me in principle, but I'd appreciate the code being more self-descriptive. Thanks.
================ Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:528 + + // Accept but warn. + if (Arg *A = Args.getLastArgNoClaim(options::OPT_mabi_EQ)) ---------------- This comment is quite terse. Could you please expand it? Maybe something like: ``` // -mabi is not a supported option for the assembler. // For GCC compatibility accept but warn. ``` ================ Comment at: clang/test/Driver/arm-abi.c:65 + +// RUN: %clang --target=arm---gnueabi -mabi=aapcs -x assembler %s -### -o /dev/null 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-ASM %s ---------------- Other tests in this file begin with a one-line explanation and I think it would be good to do the same here. Maybe something like: ``` // The combination of -x assember & -mabi is not supported but for GCC compatibility should be accepted with a warning ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153691/new/ https://reviews.llvm.org/D153691 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
