Tests and include paths

Some of the tests have #includes which can be problematic across platforms.

Part of the difficulty is in how the default include paths are done now.

Setting C_INCLUDE_PATH, CPLUS_INCLUDE_PATH, OBJCPLUS_INCLUDE_PATH, and OBJC_INCLUDE_PATH to the host compiler’s includes directory allows the compiler to find the include files, except that in running the tests with the Python script, the environment variable doesn’t seem to be getting through. It did seem to work while running the compiter under the debugger or directly from the command line, however.

The InitHeaderSearch.cpp file sets up a hard-coded path for VS9 on a g: drive, which of course, is not likely to match most users. Perhaps also adding:

C:\Program Files\Microsoft Visual Studio 9.0\VC\INCLUDE
C:\Program Files\Microsoft Visual Studio 8\VC\include

might match most users, since these are the default installation paths. And while we’re at it, throw in Cywin’s path:

C:\cygwin\usr\include

I can give you a patch for this, if you want. After adding this, the failed test count went from 230 to 204.

Or, it wouldn’t be too hard to use the environment variables VS80COMNTOOLS and VS90COMNTOOLS to look for them, or perhaps something from the registry. Shall I try this and submit a patch for this instead?

However, a key issue I want to get to in this posting is that the compiler might not be ready for everything in the host compiler’s includes, specifically both Visual Studio and Cygwin in this case, as unexpected messages arising from compiling these files are making some tests fail.

My question is, do we really want to #include standard headers in the tests?

Or, if we need certain definitions from them, should we perhaps create a set of canonical headers with just the definitions needed for the tests and include those instead, changing the #include’s in the tests to be relative, to make it obvious?

Or, what other solutions might there be? Because of licensing, I’m guessing you couldn’t just check-in some GNU headers from a platform deemed the reference standard.

-John

Hi John,

These are my opinions:

1. WIth regard to the first problem, this is a configuration problem
(or perhaps an autodetection problem). The compiler should not have
the giant union of all possible include paths, it should be configured
for the include paths that match the system. For now since we do have
the giant union, I have no problem adding some more standard MSVC
paths.

2. I think using standard headers in tests is fine. After all, users
of a C compiler probably want #include <stdlib.h> to work, for
example.

- Daniel

Here's a patch for using environment variables to set up default
include paths for Visual Studio.

I know it's not optimal yet, but I think it's an improvement over just
hard-coding paths.

I still put in some hard-coded paths if the environment variables are
not found. It seems that when run under the Python test scripts, the
environment is lost. Does anyone know anything about this? Is it
possible to tell Python to pass through the environment?

-John

win32_include_paths.patch (2.33 KB)

Sorry, I made a mistake. Please disregard the previous patch and use
this version instead. I forgot to trace this in a debugger first
before submitting it. The previous one lacked the "VC" path.

The issue about the environment variables and Python is still open, though.

-John

win32_include_paths.patch (3.38 KB)

I've been looking into the problem with using grep with search strings
with double-quotes.

I wrote a little C program that just prints out its arguments, so I
could see what grep was possibly seeing (though it's argument parsing
internally might be different, being an MSYS program).

From mangle.c, the script is given as:

RUN: dsparg '@"\\01foo"' %t

In the test report output:

Command 3: "c:\tools\bin\dsparg.EXE" "@"\\01foo""
"C:\Tools\llvm\tools\clang\test\Output\CodeGen\mangle.c.tmp"
Command 3 Result: 1
Command 3 Output:
argc = 3
argv[0] = [c:\tools\bin\dsparg.EXE]
argv[1] = [@"\\01foo"]
argv[2] = [C:\Tools\llvm\tools\clang\test\Output\CodeGen\mangle.c.tmp]

So, it appears that the search string is getting to the program, but
without the single quotes.

I tried running grep directly from the Windows command line, and this
is the only thing that worked:

C:\Tools\llvm\tools\clang\test\Output\CodeGen>grep '@"\\\\01foo"' mangle.c.tmp
@"\01foo" = common global i32 0, align 4 ; <i32*> [#uses=2]
        %tmp = load i32* @"\01foo" ; <i32> [#uses=1]
        %tmp1 = load float* bitcast (i32* @"\01foo" to float*)
; <float> [#uses=1]

The grep program (coming from MSYS) appears to be confused by the
embedded quotes, so I'm not sure how to best deal with this. Some
possibilities:

1. Find a different grep that will work.
2. Write or hack a specialized grep to use in the problematic places.
3. Make the Python script do the grep.
4. Limit the search string to avoid the double-quotes or other
problematic characters. (I.e. look for just \\01foo in the above
case).
5. Change the test case to avoid having to search for problematic
things. (Probably not an option here, since that format seems common
in the LLVM code.)
6. Switch to a test mechanism that just compares the output file to a
reference file.

-John

It’s very hard to pass arbitrary strings through the Windows CreateProcess API unmolested; Python may not be doing the right thing on your behalf. (.NET doesn’t, believe it or not.) This is some Managed C++ to implement the CreateProcess escape algorithm for a single argument:

void EscapeArg(StringBuilder %sb, String ^arg) {
// This looks utterly inane, but is correct.
unsigned quoteCount = 0;
for (int i = 0, e = arg->Length; i != e; ++i) {
wchar_t c = (*arg)[i];
switch (c) {
case L’"’:
for (; quoteCount; --quoteCount)
sb.Append(L"\\");
sb.Append(L’\’);
sb.Append(c);
break;
case L’\’:
++quoteCount;
break;
default:
for (; quoteCount; --quoteCount)
sb.Append(L’\’);
sb.Append(c);
break;
}
}
for (; quoteCount; --quoteCount)
sb.Append(L"\\");
}

This is quite dissimilar to the algorithms applicable to Unix shells. (It’s also undocumented, perhaps because it’s insane.)