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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
