[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Cullen Rhodes via Phabricator via cfe-commits]

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