v.g.vassilev added a comment. In D41416#4496389 <https://reviews.llvm.org/D41416#4496389>, @ChuanqiXu wrote:
> In D41416#4496293 <https://reviews.llvm.org/D41416#4496293>, @v.g.vassilev > wrote: > >> In D41416#4492367 <https://reviews.llvm.org/D41416#4492367>, @v.g.vassilev >> wrote: >> >>> Address most of the comments. I will need some help with the >>> `Modules/pr60085.cppm` failure. I suspect we pass to CodeGen some >>> instantiation by iterator index and that does not work as the >>> instantiations are lazily triggered upon use now. >> >> In fact it looks like it fails exactly in the same way as described in the >> original report https://github.com/llvm/llvm-project/issues/60085. The >> commit message in https://github.com/llvm/llvm-project/commit/78e48977a6e67 >> hints at the fact that the issue was gone surprisingly. I suspect that test >> case stopped reproducing the underlying issue by chance. That probably means >> that this patch is not breaking anything but exposing an underlying >> problem... > > Got it. I understand it is frustrating to fix an additional bug during > development. Also it is generally not good to revert a valid test case. I agree. Here is a reduced version of the same test case: // RUN: rm -rf %t // RUN: mkdir %t // RUN: split-file %s %t // // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \ // RUN: -emit-module-interface -o %t/d.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \ // RUN: -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \ // RUN: -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \ // RUN: -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \ // RUN: -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \ // RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm // // Use -fmodule-file=<module-name>=<BMI-path> // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \ // RUN: -emit-module-interface -o %t/d.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \ // RUN: -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \ // RUN: -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \ // RUN: -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \ // RUN: -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \ // RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm //--- d.cppm export module d; export template<typename> struct integer { using type = int; static constexpr auto value() { return 0; } }; export constexpr void ddd(auto const v) { v.value(); } //--- c.cppm export module c; import d; int use = integer<int>().value(); //--- b.cppm export module b; import d; using use = integer<int>::type; //--- a.cppm export module a; import d; export void a() { ddd(integer<int>()); } > Are you in a hurry to land this recently? (e.g., land this for clang17) If > not, I suggest to wait for me to fix the underlying issue after clang17 gets > released. And if I take too long to fix it (e.g, 9.30), given the speciality > of the patch, I think we can revert that test case and land this one that > time. How do you feel about the idea? We are not in hurry - this patch was developed in 2017 :) It can probably wait a few more years ;) I had some spare cycles to work on it. That's all. I do not think we can land it without asking @rsmith or @dblaikie to test it on their internal builds to make sure things are not regressing performance... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41416/new/ https://reviews.llvm.org/D41416 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
