[PATCH] D90874: [test] Properly test -Werror-implicit-function-declaration and -Wvec-elem-size

Thu Nov 5 17:59:25 PST 2020

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.

================
Comment at: clang/test/Sema/vecshift.c:3-4
+// RUN: %clang_cc1 -fsyntax-only -DEXT -DERR -verify=expected,vecelemsize %s
+// RUN: %clang_cc1 -fsyntax-only -DERR -verify %s -Wno-vec-elem-size
+// RUN: %clang_cc1 -fsyntax-only -DEXT -DERR -verify %s -Wno-vec-elem-size

----------------
dblaikie wrote:
> MaskRay wrote:
> > dblaikie wrote:
> > > The previous version of the test had this without -DERR, right? Why the change here?
> > I think the previous version was wrongly structured. It probably wanted to test what -Wvec-elem-size checks but Wno-error=vec-elem-size would not check anything.
> Not sure I follow - could you walk me through it in a bit more detail/few more steps?
> 
> It seems surprising to me that all 4 check lines this test has as you're proposing have `-DERR`, if that's correct, why have it a as a preprocessor variable (ie: all the non-ERR parts of the test are dead in this proposed version, it seems)? But the test as committed in 9941ca8af6b4c39fd0b9e47dc7e593d884b55710 tested both with `-DERR` and without it.
> 
> 
OK, I misunderstood the intention of the original test. 

-Wvec-elem-size is an error. To test that the error can be downgraded to a warning, -Wno-error=vec-elem-size is needed.

Fixed the RUN lines.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90874