MaskRay added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:10 #include "RISCV.h" +#include "../Clang.h" #include "ToolChains/CommonArgs.h" ---------------- compnerd wrote: > This feels like a layering violation. I suppose that as long as modular > builds are okay ... lib/Driver/ToolChains/Arch does not form a module or library. Arch/ is just a subdirectory containing these arch-specific files. So I think this is fine. ================ Comment at: clang/test/Driver/riscv-features.c:40 +// RUN: not %clang -c --target=riscv64-linux-gnu -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=ERR-SPLIT-DWARF +// RUN: not %clang -c --target=riscv64 -gsplit-dwarf=single %s 2>&1 | FileCheck %s --check-prefix=ERR-SPLIT-DWARF +// RUN: %clang -### -c --target=riscv64 -mno-relax -g -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=SPLIT-DWARF ---------------- compnerd wrote: > Did you mean to add `-###` to these two? No. In the presence of errors, without `-###` the clang command exists with 1 and does not write to the output. I feel that this behavior is better for error testing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130190/new/ https://reviews.llvm.org/D130190 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits