[PATCH] D158948: [clang][ASTImporter] Add import of type-related nodes

Fri Sep 8 19:00:50 PDT 2023

haowei added a comment.

In D158948#4642475 <https://reviews.llvm.org/D158948#4642475>, @danix800 wrote:

> In D158948#4642422 <https://reviews.llvm.org/D158948#4642422>, @haowei wrote:
>
>> Looking at https://github.com/llvm/llvm-project/blob/37a20cc68f545647e614c5ba4ae311dc3fd277e9/clang/lib/Testing/CommandLineArgs.cpp#L47, this is were the unreachable code was hit. Is it the intended behavior?
>>
>> Shall I enable `ImportMatrixType` and disable `ImportOpenCLPipe` tests so we can update the GoogleTest in LLVM?
>
> I'm not familiar with GoogleTest so I wasn't expecting this to happen.
>
> To enabling `ImportOpenCLPipe` we might use `"-x", "cl", "-cl-no-stdinc", "-cl-std=CL2.0"` here <https://github.com/llvm/llvm-project/blob/37a20cc68f545647e614c5ba4ae311dc3fd277e9/clang/lib/Testing/CommandLineArgs.cpp#L47>.
>
> See also https://reviews.llvm.org/D158872 for some extra context of both testing with `ImportMatrixType` and `ImportOpenCLPipe`.

In a nutshell, `ImportOpenCLPipe` wasn't enabled after your patch, that is why the test failure was unnoticed. You have to add

  INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportOpenCLPipe,
                           DefaultTestValuesForRunOptions);

in the same namespace like other tests in this file to let it run. I am very unfamiliar with Clang AST codebase so sorry I don't really know what changes you need to make this test pass. At current state, this test will lead to unreachable code error.

https://github.com/llvm/llvm-project/pull/65823 is my attempt to roll GoogleTest 1.14.0 to bring some upstream bug fixes and improvements to LLVM. Patch https://github.com/llvm/llvm-project/pull/65823/commits/ff67955ecbafcf847e11188c44e8dd070899b0b6 attempts to enable `ImportMatrixType` and explicitly disable `ImportOpenCLPipe` so the `check-clang` step can pass under GoogleTest v1.14.0

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158948