Author: Chuanqi Xu Date: 2024-01-23T16:19:51+08:00 New Revision: ba1e84fb8f45e102f40f409fcfe9b420fbf9fb70
URL: https://github.com/llvm/llvm-project/commit/ba1e84fb8f45e102f40f409fcfe9b420fbf9fb70 DIFF: https://github.com/llvm/llvm-project/commit/ba1e84fb8f45e102f40f409fcfe9b420fbf9fb70.diff LOG: [C++20] [Modules] Handle inconsistent deduced function return type from importing modules Close https://github.com/llvm/llvm-project/issues/78830 Close https://github.com/llvm/llvm-project/issues/60085 The direct reason of the issues is that in a module unit, the return type of a function is deduced to a concrete type (e.g., int) but in the other module unit, the return type of the same function is not deduced yet (e.g, auto). Then when we importing the 2 modules, the later function is selected but the code generator doesn't know how to generate the auto type. So here is the crash. The tricky part here is that, when the ASTReader reads the second unreduced function, it finds the reading function has the same signature with the already read deduced one and they have the same ODRHash. So that the ASTReader skips loading the function body of the unreduced function then the code generator can't infer the undeduced type like it usually can. Also this is generally fine for functions without deducing type since it is sufficient to emit a function call without the function body. Also in fact, we've already handled the case that the functon has deduced type and its deducing state is inconsist in different modules: https://github.com/llvm/llvm-project/blob/3ea92ea2f9d236569f82825cdba6d59bcc22495c/clang/lib/Serialization/ASTReader.cpp#L9531-L9544 and https://github.com/llvm/llvm-project/blob/3ea92ea2f9d236569f82825cdba6d59bcc22495c/clang/lib/Serialization/ASTReaderDecl.cpp#L3643-L3647. We've handled the case: (1) If we read the undeduced functions first and read the deduced functions later, the compiler will propagate the deduced type info for redecls in the end of the reading. (2) If we read the deduced functions first and read the undeduced functions later, we will propagae the deduced type info when we **complete the redecl chain**. However, in the reporting issues, we're in the second case and reproducer didn't trigger the action to complete the redecl chain. So here is the crash. Then it is obvious how should fix the problem. We should complete the redecl chain for undeduced function types in the end of the reading for the second case. Added: clang/test/Modules/pr60085.cppm clang/test/Modules/pr78830.cppm Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Serialization/ASTReader.h clang/lib/Serialization/ASTReader.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 93eecbe0e1363de..01c4ee97662b611 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1049,6 +1049,11 @@ Bug Fixes to C++ Support - Set the ``__cpp_auto_cast`` feature test macro in C++23 mode. +- Fix crash for inconsistent deducing state of function return types + in importing modules. + Fixes (`#78830 <https://github.com/llvm/llvm-project/issues/78830>`_) + Fixes (`#60085 <https://github.com/llvm/llvm-project/issues/60085>`_) + Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed an import failure of recursive friend class template. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 21d791f5cd89a2e..dd1451bbf2d2c9f 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -550,6 +550,10 @@ class ASTReader /// declaration and the value is the deduced return type. llvm::SmallMapVector<FunctionDecl *, QualType, 4> PendingDeducedTypeUpdates; + /// Functions has undededuced return type and we wish we can find the deduced + /// return type by iterating the redecls in other modules. + llvm::SmallVector<FunctionDecl *, 4> PendingUndeducedFunctionDecls; + /// Declarations that have been imported and have typedef names for /// linkage purposes. llvm::DenseMap<std::pair<DeclContext *, IdentifierInfo *>, NamedDecl *> diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index fe8782a3eb9e7cc..fecd94e875f671a 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9534,12 +9534,21 @@ void ASTReader::finishPendingActions() { auto *FD = PendingDeducedFunctionTypes[I].first; FD->setType(GetType(PendingDeducedFunctionTypes[I].second)); - // If we gave a function a deduced return type, remember that we need to - // propagate that along the redeclaration chain. - auto *DT = FD->getReturnType()->getContainedDeducedType(); - if (DT && DT->isDeduced()) - PendingDeducedTypeUpdates.insert( - {FD->getCanonicalDecl(), FD->getReturnType()}); + if (auto *DT = FD->getReturnType()->getContainedDeducedType()) { + // If we gave a function a deduced return type, remember that we need to + // propagate that along the redeclaration chain. + if (DT->isDeduced()) { + PendingDeducedTypeUpdates.insert( + {FD->getCanonicalDecl(), FD->getReturnType()}); + continue; + } + + // The function has undeduced DeduceType return type. We hope we can + // find the deduced type by iterating the redecls in other modules + // later. + PendingUndeducedFunctionDecls.push_back(FD); + continue; + } } PendingDeducedFunctionTypes.clear(); @@ -10105,6 +10114,13 @@ void ASTReader::FinishedDeserializing() { getContext().adjustDeducedFunctionResultType(Update.first, Update.second); } + + auto UDTUpdates = std::move(PendingUndeducedFunctionDecls); + PendingUndeducedFunctionDecls.clear(); + // We hope we can find the deduced type for the functions by iterating + // redeclarations in other modules. + for (FunctionDecl *UndeducedFD : UDTUpdates) + (void)UndeducedFD->getMostRecentDecl(); } if (ReadTimer) diff --git a/clang/test/Modules/pr60085.cppm b/clang/test/Modules/pr60085.cppm new file mode 100644 index 000000000000000..fd6fd914a543c3f --- /dev/null +++ b/clang/test/Modules/pr60085.cppm @@ -0,0 +1,85 @@ +// 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 -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \ +// RUN: -emit-module-interface -o %t/b.pcm -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \ +// RUN: -emit-module-interface -o %t/a.pcm -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \ +// RUN: -S -emit-llvm -disable-llvm-passes -o - -fprebuilt-module-path=%t \ +// RUN: | FileCheck %t/a.cppm + +//--- d.cppm +export module d; + +export template<typename> +struct integer { + using type = int; + + static constexpr auto value() { + return 0; + } + + friend constexpr void f(integer const x) { + x.value(); + } +}; + +export constexpr void ddd(auto const value) { + f(value); +} + + +template<typename T> +constexpr auto dd = T(); + +export template<typename T> +constexpr auto d() { + dd<T>; +} + +//--- c.cppm +export module c; + +import d; + +template<typename T> +auto cc = T(); + +auto c() { + cc<integer<int>>; + integer<int>().value(); +} + +//--- b.cppm +export module b; + +import d; + +auto b() { + integer<int>::type; +} + +//--- a.cppm +export module a; + +import b; +import c; +import d; + +constexpr void aa() { + d<integer<unsigned>>(); + ddd(integer<int>()); +} + +export extern "C" void a() { + aa(); +} + +// Checks that we emit the IR successfully. +// CHECK: define{{.*}}@a( diff --git a/clang/test/Modules/pr78830.cppm b/clang/test/Modules/pr78830.cppm new file mode 100644 index 000000000000000..a3b1a8021ebea83 --- /dev/null +++ b/clang/test/Modules/pr78830.cppm @@ -0,0 +1,56 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/Type.cppm -emit-module-interface -o \ +// RUN: %t/MyVec-Type.pcm -triple=x86_64-linux-gnu +// RUN:%clang_cc1 -std=c++20 %t/Vec.cppm -emit-module-interface -o \ +// RUN: %t/MyVec-Vec.pcm -fmodule-file=MyVec:Type=%t/MyVec-Type.pcm \ +// RUN: -triple=x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++20 %t/Vec2.cppm -emit-module-interface -o \ +// RUN: %t/MyVec-Vec2.pcm -fmodule-file=MyVec:Type=%t/MyVec-Type.pcm \ +// RUN: -triple=x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++20 %t/Calculator.cppm -emit-module-interface -o \ +// RUN: %t/MyVec-Calculator.pcm -fmodule-file=MyVec:Vec=%t/MyVec-Vec.pcm \ +// RUN: -fmodule-file=MyVec:Vec2=%t/MyVec-Vec2.pcm \ +// RUN: -fmodule-file=MyVec:Type=%t/MyVec-Type.pcm \ +// RUN: -triple=x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++20 %t/MyVec-Calculator.pcm -S -emit-llvm \ +// RUN: -fmodule-file=MyVec:Vec=%t/MyVec-Vec.pcm \ +// RUN: -fmodule-file=MyVec:Vec2=%t/MyVec-Vec2.pcm \ +// RUN: -fmodule-file=MyVec:Type=%t/MyVec-Type.pcm \ +// RUN: -triple=x86_64-linux-gnu -o - \ +// RUN: | FileCheck %t/Calculator.cppm + +//--- Type.cppm +export module MyVec:Type; + +template <class T> struct Size { + auto total() const { return 1; } +}; + +//--- Vec.cppm +export module MyVec:Vec; +import :Type; + +int size_ = Size<int>().total(); + +//--- Vec2.cppm +export module MyVec:Vec2; +import :Type; + +struct Vec2 { + Size<int> size_; +}; + +//--- Calculator.cppm +export module MyVec:Calculator; + +import :Vec; +import :Vec2; + +auto Calculate() { return Size<int>().total(); }; + +// Check the emitted module initializer to make sure we generate the module unit +// successfully. +// CHECK: @_ZW5MyVec9Calculatev _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
