c-rhodes added inline comments.
================ Comment at: clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c:1-32 +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \ +// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \ +// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty + +// CHECK-NOT: warning + +#include <arm_sve.h> ---------------- joechrisellis wrote: > c-rhodes wrote: > > I wonder if we can just add `-Wconversion` to the existing Sema tests? > > * clang/test/Sema/attr-arm-sve-vector-bits.c > > * clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp > > > `clang/test/Sema/attr-arm-sve-vector-bits.c` contains a lot of `// > expected-error ...` and such, so I'm reluctant to make changes to that > because it seems a little weird to mix up expected errors and non-expected > warnings. > > I suppose we could cat this test into `attr-arm-sve-vector-bits.cpp`, though, > if you would prefer? 🙂 > `clang/test/Sema/attr-arm-sve-vector-bits.c` contains a lot of `// > expected-error ...` and such, so I'm reluctant to make changes to that > because it seems a little weird to mix up expected errors and non-expected > warnings. > I sort of see what you mean but unless there's a good reason (which I can't really think of) then I think it's preferential to introducing another test for this attribute. There's already a bunch of tests with this exact logic being tested here the only difference is `-Wconversion` is on. To be honest I don't have strong feelings about it so it can stay as a separate test if you wish, but if it does I think this can be reduced to a single test for predicates and one extra test for a data vector like int8 or something. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97053/new/ https://reviews.llvm.org/D97053 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
