Re: r284793 - Remove 24 instances of 'REQUIRES: shell'

2016-10-20 Thread Reid Kleckner via cfe-commits
On Thu, Oct 20, 2016 at 4:38 PM, Robinson, Paul 
wrote:

> Is there any expectation that they _should_ (eventually) work on Windows?
>

Yes, LLVM is a cross-platform project, and there is an expectation that
developers will attempt to write portable code. It seems like there is some
dependence in these tests on the host compiler, the order of fields in a
struct changes if the host compiler is MSVC.

Also, I noticed I broke a bot. I reverted some changes, hopefully that's
enough.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


RE: r284793 - Remove 24 instances of 'REQUIRES: shell'

2016-10-20 Thread Robinson, Paul via cfe-commits

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Reid Kleckner via cfe-commits
> Sent: Thursday, October 20, 2016 4:12 PM
> To: cfe-commits@lists.llvm.org
> Subject: r284793 - Remove 24 instances of 'REQUIRES: shell'
> 
> Author: rnk
> Date: Thu Oct 20 18:11:45 2016
> New Revision: 284793
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=284793&view=rev
> Log:
> Remove 24 instances of 'REQUIRES: shell'
> 
> Tests fall into one of the following categories:
> 
> - The requirement was unnecessary
> 
> - Additional quoting was required for backslashes in paths (see "sed -e
>   's/\\//g'") in the sanitizer tests.
> 
> - OpenMP used 'REQUIRES: shell' as a proxy for the test failing on
>   Windows. Those tests fail there reliably, so use XFAIL instead.

Is there any expectation that they _should_ (eventually) work on Windows?
If not, seems like UNSUPPORTED would be clearer (and more efficient).
--paulr

> 
> I tried not to remove shell requirements that were added to suppress
> flaky test failures, but if I screwed up, we can add it back as needed.
> 
> Modified:
> cfe/trunk/test/Analysis/plist-html-macros.c
> cfe/trunk/test/CodeGen/address-safety-attr.cpp
> cfe/trunk/test/CodeGen/asan-globals.cpp
> cfe/trunk/test/CodeGen/sanitize-init-order.cpp
> cfe/trunk/test/CodeGen/sanitize-thread-attr.cpp
> cfe/trunk/test/CodeGen/ubsan-blacklist.c
> cfe/trunk/test/Driver/fsanitize-blacklist.c
> cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c
> cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> cfe/trunk/test/Modules/ModuleDebugInfo.m
> cfe/trunk/test/Modules/dependency-dump-dependent-module.m
> cfe/trunk/test/Modules/empty.modulemap
> cfe/trunk/test/Modules/explicit-build-extra-files.cpp
> cfe/trunk/test/Modules/prune.m
> cfe/trunk/test/Modules/signal.m
> cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp
> cfe/trunk/test/OpenMP/task_private_codegen.cpp
> cfe/trunk/test/OpenMP/taskloop_firstprivate_codegen.cpp
> cfe/trunk/test/OpenMP/taskloop_lastprivate_codegen.cpp
> cfe/trunk/test/OpenMP/taskloop_private_codegen.cpp
> cfe/trunk/test/OpenMP/taskloop_simd_firstprivate_codegen.cpp
> cfe/trunk/test/OpenMP/taskloop_simd_lastprivate_codegen.cpp
> cfe/trunk/test/OpenMP/taskloop_simd_private_codegen.cpp
> cfe/trunk/test/PCH/debug-info-pch-path.c
> 
> Modified: cfe/trunk/test/Analysis/plist-html-macros.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plist-
> html-macros.c?rev=284793&r1=284792&r2=284793&view=diff
> ==
> 
> --- cfe/trunk/test/Analysis/plist-html-macros.c (original)
> +++ cfe/trunk/test/Analysis/plist-html-macros.c Thu Oct 20 18:11:45 2016
> @@ -1,12 +1,11 @@
> -// REQUIRES: shell
>  // RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
>  // (sanity check)
> 
>  // RUN: rm -rf %t.dir
>  // RUN: mkdir -p %t.dir
>  // RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-
> output=plist-html -o %t.dir/index.plist %s
> -// RUN: ls %t.dir | grep \\.html | count 1
> -// RUN: grep \\.html %t.dir/index.plist | count 1
> +// RUN: ls %t.dir | grep '\.html' | count 1
> +// RUN: grep '\.html' %t.dir/index.plist | count 1
> 
>  // This tests two things: that the two calls to null_deref below are
> coalesced
>  // into a single bug by both the plist and HTML diagnostics, and that the
> plist
> 
> Modified: cfe/trunk/test/CodeGen/address-safety-attr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/address-
> safety-attr.cpp?rev=284793&r1=284792&r2=284793&view=diff
> ==
> 
> --- cfe/trunk/test/CodeGen/address-safety-attr.cpp (original)
> +++ cfe/trunk/test/CodeGen/address-safety-attr.cpp Thu Oct 20 18:11:45
> 2016
> @@ -9,12 +9,11 @@ int DefinedInDifferentFile(int *a);
>  // RUN: echo "fun:*BlacklistedFunction*" > %t.func.blacklist
>  // RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -emit-llvm -o -
> %s -include %t.extra-source.cpp -fsanitize=address -fsanitize-
> blacklist=%t.func.blacklist | FileCheck -check-prefix=BLFUNC %s
> 
> -// RUN: echo "src:%s" > %t.file.blacklist
> +// The blacklist file uses regexps, so escape backslashes, which are
> common in
> +// Windows paths.
> +// RUN: echo "src:%s" | sed -e 's/\\//g' > %t.file.blacklist
>  // RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -emit-llvm -o -
> %s -include %t.extra-source.cpp -fsanitize=address -fsanitize-
> blacklist=%t.file.blacklist | FileCheck -check-prefix=BLFILE %s
> 
> -// FIXME: %t.file.blacklist is like
> "src:x:\path\to\clang\test\CodeGen\address-safety-attr.cpp"
> -// REQUIRES: shell
> -
>  // The sanitize_address attribute should be attached to functions
>  // when AddressSanitizer is enabled, unless no_sanitize_address attribute
>  // is present.
> 
> Modified: cfe/trunk/test