Skip to content

Commit

Permalink
[C++20] [Modules] Handle inconsistent deduced function return type fr…
Browse files Browse the repository at this point in the history
…om importing modules

Close #78830
Close #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.
  • Loading branch information
ChuanqiXu9 committed Jan 23, 2024
1 parent 7da7695 commit ba1e84f
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 6 deletions.
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *>
Expand Down
28 changes: 22 additions & 6 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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)
Expand Down
85 changes: 85 additions & 0 deletions clang/test/Modules/pr60085.cppm
Original file line number Diff line number Diff line change
@@ -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(
56 changes: 56 additions & 0 deletions clang/test/Modules/pr78830.cppm
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit ba1e84f

Please sign in to comment.