Skip to content

Commit

Permalink
[Modules] Fix creating fake definition data for lambdas.
Browse files Browse the repository at this point in the history
During reading C++ definition data for lambda we can access
CXXRecordDecl representing lambda before we finished reading the
definition data. This can happen by reading a captured variable which is
VarDecl, then reading its decl context which is CXXMethodDecl `operator()`,
then trying to merge redeclarable methods and accessing
enclosing CXXRecordDecl. The call stack looks roughly like

    VisitCXXRecordDecl
      ReadCXXRecordDefinition
        VisitVarDecl
          VisitCXXMethodDecl
            mergeRedeclarable
              getPrimaryContextForMerging

If we add fake definition data at this point, later we'll hit the assertion

    Assertion failed: (!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?"), function MergeDefinitionData, file clang/lib/Serialization/ASTReaderDecl.cpp, line 1675.

The fix is to assign definition data before reading it. Fixes PR32556.

rdar://problem/37461072

Reviewers: rsmith, bruno

Reviewed By: rsmith

Subscribers: cfe-commits, jkorous-apple, aprantl

Differential Revision: https://reviews.llvm.org/D43494

llvm-svn: 328153
  • Loading branch information
vsapsai committed Mar 21, 2018
1 parent 1d0832d commit 8a5943f
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 10 deletions.
22 changes: 12 additions & 10 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Expand Up @@ -1784,29 +1784,31 @@ void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) {
else
DD = new (C) struct CXXRecordDecl::DefinitionData(D);

CXXRecordDecl *Canon = D->getCanonicalDecl();
// Set decl definition data before reading it, so that during deserialization
// when we read CXXRecordDecl, it already has definition data and we don't
// set fake one.
if (!Canon->DefinitionData)
Canon->DefinitionData = DD;
D->DefinitionData = Canon->DefinitionData;
ReadCXXDefinitionData(*DD, D);

// We might already have a definition for this record. This can happen either
// because we're reading an update record, or because we've already done some
// merging. Either way, just merge into it.
CXXRecordDecl *Canon = D->getCanonicalDecl();
if (Canon->DefinitionData) {
// We might already have a different definition for this record. This can
// happen either because we're reading an update record, or because we've
// already done some merging. Either way, just merge into it.
if (Canon->DefinitionData != DD) {
MergeDefinitionData(Canon, std::move(*DD));
D->DefinitionData = Canon->DefinitionData;
return;
}

// Mark this declaration as being a definition.
D->IsCompleteDefinition = true;
D->DefinitionData = DD;

// If this is not the first declaration or is an update record, we can have
// other redeclarations already. Make a note that we need to propagate the
// DefinitionData pointer onto them.
if (Update || Canon != D) {
Canon->DefinitionData = D->DefinitionData;
if (Update || Canon != D)
Reader.PendingDefinitions.insert(D);
}
}

ASTDeclReader::RedeclarableResult
Expand Down
4 changes: 4 additions & 0 deletions clang/test/Modules/Inputs/self-referencing-lambda/a.h
@@ -0,0 +1,4 @@
void f() {
int x = 0;
auto q = [xm = x]{};
}
@@ -0,0 +1,4 @@
module "a.h" {
header "a.h"
export *
}
5 changes: 5 additions & 0 deletions clang/test/Modules/self-referencing-lambda.cpp
@@ -0,0 +1,5 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/self-referencing-lambda %s -verify -emit-obj -o %t2.o
// expected-no-diagnostics

#include "a.h"

0 comments on commit 8a5943f

Please sign in to comment.