Add import of type-related nodes:
- bitIntType
- constantMatrixType
- dependentAddressSpaceType
- dependentBitIntType
- dependentSizedMatrixType
- dependentVectorType
- objcTypeParamDecl
- objcTypeParamType
- pipeType
- vectorType
Differential D158948
[clang][ASTImporter] Add import of type-related nodes danix800 on Aug 26 2023, 8:49 PM. Authored by
Details Add import of type-related nodes:
Diff Detail Event Timeline
Comment Actions Pull in local matchers for testcases, since https://reviews.llvm.org/D158872 is abandoned. Comment Actions Hi I am working on updating GoogleTest in LLVM to v1.14.0 and during my local testing, I noticed that this patch added 2 new tests ImportMatrixType and ImportOpenCLPipe through TEST_P calls, however, both tests were not instantiated by INSTANTIATE_TEST_SUITE_P so they were not ran during check-clang. This will causes test failures when using GoogleTest v1.14.0 since it now explicitly fail when an INSTANTIATE_TEST_SUITE_P is not paired with an TEST_P I manually added INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportMatrixType, DefaultTestValuesForRunOptions); INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportOpenCLPipe, DefaultTestValuesForRunOptions); to ASTImporterTest.cpp and reran the clang test and it failed with error message: [ RUN ] ParameterizedTests/ImportOpenCLPipe.ImportPipeType/0 Not implemented yet! UNREACHABLE executed at clang/lib/Testing/CommandLineArgs.cpp:47! Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 ASTTests 0x00005565e9e71d18 -- exit: -6 -- 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? Comment Actions 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. See also https://reviews.llvm.org/D158872 for some extra context of both testing with ImportMatrixType and ImportOpenCLPipe. Comment Actions @haowei I noticed https://github.com/llvm/llvm-project/pull/65823, is it related to your work? Comment Actions 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 |
At cases like this (many imported values) importChecked can be used, like at VisitDependentSizedArrayType.