[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

Ella Ma via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 22 02:13:16 PST 2021


OikawaKirie added a comment.

In D102669#3206089 <https://reviews.llvm.org/D102669#3206089>, @steakhal wrote:

> 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.

I do not think it is this patch that breaks the functionality on M1 <https://reviews.llvm.org/M1>, as it depends on the *on-demand-parsing* feature that is not tested on M1 <https://reviews.llvm.org/M1> currently.

> We should add a test-case demonstrating the problem with M1 <https://reviews.llvm.org/M1> with a given configuration.

If I got it correct (it is on-demand-parsing that triggers the problem), this problem can be triggered by enabling the test case of D75665 <https://reviews.llvm.org/D75665> on M1 <https://reviews.llvm.org/M1>.

> 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.

Maybe currently a simpler way is trying to use AST dump to load the external TU to be imported, rather than on-demand-parsing, which can make us fix this failure with the test case still enabled on M1 <https://reviews.llvm.org/M1>.

I will have a series of tests on my concerns later, and I will reply with my results if I can find something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669



More information about the cfe-commits mailing list