alekseyshl added inline comments.

================
Comment at: test/Driver/asan.c:12
-// RUN: %clang -O2 -target aarch64-unknown-linux -fsanitize=hwaddress %s -S 
-emit-llvm -o - | FileCheck %s --check-prefix=CHECK-HWASAN
-// RUN: %clang -O3 -target aarch64-unknown-linux -fsanitize=hwaddress %s -S 
-emit-llvm -o - | FileCheck %s --check-prefix=CHECK-HWASAN
 // Verify that -fsanitize={address,kernel-address} invoke ASan and KASan 
instrumentation.
----------------
eugenis wrote:
> alekseyshl wrote:
> > eugenis wrote:
> > > Don't replace existing tests!
> > > Add new ones.
> > > 
> > I am not replacing them, I am making it consistent with all other 
> > sanitizers, which also support many more platforms than just 
> > x86_64-linux-gnu or *-unknown-linux, but have tests for that one platform 
> > only.
> I don't think this is a good argument for changing existing tests. It does 
> not add any new coverage, and in this particular example it is not even 
> consistent with surrounding RUN lines - those use i386 triple. It simply adds 
> noise to code history.
> 
> If you want to test that hwasan supports x86_64 triple - add a new test case.
> 
Ok, this file, I'm ambivalent about, it is not consistent already, but the 
other ones I do feel quite strongly about, they are become more consistent and 
consistency is good.


Repository:
  rC Clang

https://reviews.llvm.org/D44745



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to