Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,12 @@ class ASTReader

/// Mapping from main decl ID to the related decls IDs.
///
/// These related decls have to be loaded right after the main decl.
/// It is required to have canonical declaration for related decls from the
/// same module as the enclosing main decl. Without this, due to lazy
/// deserialization, canonical declarations for the main decl and related can
/// be selected from different modules.
/// The key is the main decl ID, and the value is a vector of related decls
/// that must be loaded immediately after the main decl. This is necessary
/// to ensure that the definition for related decls comes from the same module
/// as the enclosing main decl. Without this, due to lazy deserialization,
/// the definition for the main decl and related decls may come from different
/// modules.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this map is used to ensure that the main declaration and its related declarations are always deserialized from the same module in the following cases:

  • Lambda inside a function declaration: The main declaration is the enclosing function, and the related declarations are the lambda declarations.
  • Defined friend declaration inside a CXXRecord declaration: The main declaration is the enclosing record, and the related declarations are the friend declarations.

Is my understanding correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right understanding. Initially it was only for lambda functions but later case with inline friend functions were identified. The mechanism in its current form is very generic so it can be used for other case if we find them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming.

I think it would be worth documenting this (it took me some time to fully understand what RelatedDeclsMap is and how it's used). Clear documentation could help others in the future.

llvm::DenseMap<GlobalDeclID, SmallVector<GlobalDeclID, 4>> RelatedDeclsMap;

struct PendingUpdateRecord {
Expand Down
11 changes: 6 additions & 5 deletions clang/lib/Serialization/ASTWriterDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -799,9 +799,9 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
}

if (D->getFriendObjectKind()) {
// For a function defined inline within a class template, we have to force
// the canonical definition to be the one inside the canonical definition of
// the template. Remember this relation to deserialize them together.
// For a friend function defined inline within a class template, we have to
// force the definition to be the one inside the definition of the template
// class. Remember this relation to deserialize them together.
if (auto *RD = dyn_cast<CXXRecordDecl>(D->getLexicalParent()))
if (RD->isDependentContext() && RD->isThisDeclarationADefinition()) {
Writer.RelatedDeclsMap[Writer.GetDeclRef(RD)].push_back(
Expand Down Expand Up @@ -1571,9 +1571,10 @@ void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
} else {
Record.push_back(0);
}
// For lambdas inside canonical FunctionDecl remember the mapping.
// For lambdas inside template functions, remember the mapping to
// deserialize them together.
if (auto FD = llvm::dyn_cast_or_null<FunctionDecl>(D->getDeclContext());
FD && FD->isCanonicalDecl()) {
FD && FD->isDependentContext() && FD->isThisDeclarationADefinition()) {
Writer.RelatedDeclsMap[Writer.GetDeclRef(FD)].push_back(
Writer.GetDeclRef(D));
}
Expand Down
92 changes: 92 additions & 0 deletions clang/test/Headers/crash-instantiated-in-scope-cxx-modules5.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// RUN: rm -fR %t
// RUN: split-file %s %t
// RUN: cd %t
// RUN: %clang_cc1 -verify -std=c++20 -Werror=uninitialized -xc++ -emit-module module.cppmap -fmodule-name=mock_resolver -o mock_resolver.pcm
// RUN: %clang_cc1 -verify -std=c++20 -Werror=uninitialized -xc++ -emit-module module.cppmap -fmodule-name=sql_internal -o sql_internal.pcm
// RUN: %clang_cc1 -verify -std=c++20 -Werror=uninitialized -xc++ -fmodule-file=mock_resolver.pcm -fmodule-file=sql_internal.pcm main.cc -o main.o

//--- module.cppmap
module "mock_resolver" {
export *
module "mock_resolver.h" {
export *
header "mock_resolver.h"
}
}

module "sql_internal" {
export *
module "sql_transform_builder.h" {
export *
header "sql_transform_builder.h"
}
}

//--- set_bits2.h
// expected-no-diagnostics
#pragma once

template <typename T>
void fwd(const T& x) {}

namespace vox::bitset {

template <typename TFunc>
void ForEachSetBit2(const TFunc&) {
fwd([](int) {
const int bit_index_base = 0;
(void)[&](int) {
int v = bit_index_base;
};
});
}

} // namespace vox::bitset

//--- sql_transform_builder.h
// expected-no-diagnostics
#pragma once

#include "set_bits2.h"

class QualifyingSet3 {
public:
void GetIndexes() const {
vox::bitset::ForEachSetBit2([]() {});
}
};

template <typename T>
void DoTransform() {
vox::bitset::ForEachSetBit2([]() {});
}

//--- mock_resolver.h
// expected-no-diagnostics
#pragma once
#include "set_bits2.h"

class QualifyingSet2 {
public:
void GetIndexes() const {
vox::bitset::ForEachSetBit2([]() {});
}
};

//--- main.cc
// expected-no-diagnostics
#include "sql_transform_builder.h"

template <typename Callable>
void get(const Callable& fn) {
fwd<Callable>(fn);
}

namespace {

void test() {
get([]() {});
DoTransform<int>();
}

} // namespace
Loading