[Lldb-commits] [PATCH] D54942: [PDB] Make PDB lit tests use the new builder

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

> Clang called from the builder shows errors on types like char16_t and char32_t. That's why enums-layout.test and typedefs.test are made to use MSVC instead;

For me enums-layout.test crashes, but I see the same behavior in typedefs.test.  This is because lld-link does not emit `S_UDT` records (note the original test was using %msvc_link, not %lld_link.  That's why it passed before).  I could probably add a new magic toolchain called clang-cl-with-msvc-linker but I think it's also fine to just keep these using cl.exe.

> The builder now doesn't support /Gy and /order options, so the old scheme is used in function-level-linking.test;

Makes sense.  Just curious, is the order file strictly necessary for this test?  `/Gy` is the same as `-ffunction-sections`, so there could be an argument to be made for exposing `--function-sections` on the command line of `build.py`.  On the other hand, there's no harm in falling back to the old mechanism here since this test isn't intended to be portable.

> The builder now doesn't support compilation of multiple sources and linking them together, so the old scheme is used for linking in func-symbols.test;

This one is important to fix.

> On Windows 32-bit LLDB can debug only 32-bit applications, and 64-bit LLDB can debug only 64-bit applications. That's why the old scheme is left in dynamic tests udt-layout.test, variables-locations.test and vbases.test. If we will always compile the tests as 32-bit (64-bit), then they will fail on the 64-bit (32-bit) platform. That's why we just assume that the user runs the tests from the same command prompt where the build was run.

Ahh, right.  I wonder if we should have something like `--arch=lldb` that means "match the architecture of LLDB".  It's not urgent for this patch, but it could be useful as a followup.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D54942