steakhal reopened this revision. steakhal added a comment. This revision is now accepted and ready to land.
In D102669#3205889 <https://reviews.llvm.org/D102669#3205889>, @OikawaKirie wrote: > In D102669#3199270 <https://reviews.llvm.org/D102669#3199270>, @steakhal > wrote: > >> We shouldn't skip mac targets. I CC ASTImporter folks, they probably have an >> M1 <https://reviews.llvm.org/M1>. > > I am not intended to ignore this problem triggered on M1 > <https://reviews.llvm.org/M1>. However, I think it is not this patch that > leads to this problem, it just triggers it. > I mean we can just disable the test case temporarily on M1 > <https://reviews.llvm.org/M1>, and fix this problem as well as enable this > patch and the one of on-demand-parsing in another patch. > I think they trigger the same problem for the same reason on M1 > <https://reviews.llvm.org/M1>. > > Besides, it seems to be the problem of `ASTUnit::LoadFromCommandLine`, rather > than the ASTImporter. Prior to this patch, it worked on M1 <https://reviews.llvm.org/M1>; after landing it broke something, so we clearly shouldn't land this. We should add a test-case demonstrating the problem with M1 <https://reviews.llvm.org/M1> with a given configuration. Then we need to track down and fix the underlying issue causing it. That should be done probably in a separate patch and add it as a parent patch to this one. If all of these are done, we can probably land both of them. ================ Comment at: clang/test/Analysis/ctu-lookup-name-with-space.cpp:13 +// RUN: -verify %s + +void importee(); ---------------- OikawaKirie wrote: > arichardson wrote: > > OikawaKirie wrote: > > > Adding this line here. > > Disabling the test on non- Linux is not a good idea IMO since it means we > > lose coverage on other platforms. My guess is that you just need to specify > > an explicit triple in the clang invocations. > AFAIK, we cannot do that. If this test case is executed on different > platforms, we cannot determine the triple ahead of time and specify it in the > invocation list. If we were to pin the triple, then each platform would emit the correct AST dumps according to that platform - ~~ cross-compilation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102669/new/ https://reviews.llvm.org/D102669 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits