[Lldb-commits] [clang] c13dd74 - Set the captures on a CXXRecordDecl representing a lambda closure type

Richard Smith via lldb-commits lldb-commits at lists.llvm.org
Fri Jun 5 16:07:25 PDT 2020


On Thu, 4 Jun 2020 at 23:45, Jonas Devlieghere via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Hey Richard,
>
> It appears this broke the lldb bots:
>
> http://lab.llvm.org:8011/builders/lldb-x86_64-debian
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/20549/
>

That's not caused by this NFC commit, it's the one after it. Thanks for the
revert.


> It's hitting an assertion in
> llvm-project/clang/include/clang/AST/DeclCXX.h:887:
>
> Assertion `(data().DefaultedCopyAssignmentIsDeleted ||
> needsOverloadResolutionForCopyAssignment()) && "copy assignment should not
> be deleted"' failed.
>
> I've reverted this commit and the subsequent one
> (c57f8a3a20540fcf9fbf98c0a73f381ec32fce2a) to turn the bots green again
> overnight. Could you please take a look tomorrow?
>

Looks like lldb is doing unusual things with special members when
generating classes directly from debug info. It can't use Sema, so it's
faking up some of what Sema would do, and needed to be extended to also
fake up the new stuff for copy assignments in this patch.

Re-landed with fix in 825e3bb58082eafa8db87a9034379b88f892ce9d.


> Thanks,
> Jonas
>
>
>
> On Thu, Jun 4, 2020 at 7:25 PM Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>>
>> Author: Richard Smith
>> Date: 2020-06-04T19:19:01-07:00
>> New Revision: c13dd74e311d2ac70dd3ea663d800307d1aa5b6b
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/c13dd74e311d2ac70dd3ea663d800307d1aa5b6b
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/c13dd74e311d2ac70dd3ea663d800307d1aa5b6b.diff
>>
>> LOG: Set the captures on a CXXRecordDecl representing a lambda closure
>> type
>> before marking it complete.
>>
>> No functionality change intended.
>>
>> Added:
>>
>>
>> Modified:
>>     clang/include/clang/AST/DeclCXX.h
>>     clang/include/clang/AST/ExprCXX.h
>>     clang/lib/AST/ASTImporter.cpp
>>     clang/lib/AST/DeclCXX.cpp
>>     clang/lib/AST/ExprCXX.cpp
>>     clang/lib/Sema/SemaLambda.cpp
>>
>> Removed:
>>
>>
>>
>>
>> ################################################################################
>> diff  --git a/clang/include/clang/AST/DeclCXX.h
>> b/clang/include/clang/AST/DeclCXX.h
>> index 3a400a778e53..856717fa0abb 100644
>> --- a/clang/include/clang/AST/DeclCXX.h
>> +++ b/clang/include/clang/AST/DeclCXX.h
>> @@ -999,6 +999,9 @@ class CXXRecordDecl : public RecordDecl {
>>      return
>> static_cast<LambdaCaptureDefault>(getLambdaData().CaptureDefault);
>>    }
>>
>> +  /// Set the captures for this lambda closure type.
>> +  void setCaptures(ArrayRef<LambdaCapture> Captures);
>> +
>>    /// For a closure type, retrieve the mapping from captured
>>    /// variables and \c this to the non-static data members that store the
>>    /// values or references of the captures.
>> @@ -1030,6 +1033,8 @@ class CXXRecordDecl : public RecordDecl {
>>                        : nullptr;
>>    }
>>
>> +  unsigned capture_size() const { return getLambdaData().NumCaptures; }
>> +
>>    using conversion_iterator = UnresolvedSetIterator;
>>
>>    conversion_iterator conversion_begin() const {
>>
>> diff  --git a/clang/include/clang/AST/ExprCXX.h
>> b/clang/include/clang/AST/ExprCXX.h
>> index 272ad138d14a..56b27d57bd5c 100644
>> --- a/clang/include/clang/AST/ExprCXX.h
>> +++ b/clang/include/clang/AST/ExprCXX.h
>> @@ -1862,10 +1862,9 @@ class LambdaExpr final : public Expr,
>>    /// Construct a lambda expression.
>>    LambdaExpr(QualType T, SourceRange IntroducerRange,
>>               LambdaCaptureDefault CaptureDefault,
>> -             SourceLocation CaptureDefaultLoc, ArrayRef<LambdaCapture>
>> Captures,
>> -             bool ExplicitParams, bool ExplicitResultType,
>> -             ArrayRef<Expr *> CaptureInits, SourceLocation ClosingBrace,
>> -             bool ContainsUnexpandedParameterPack);
>> +             SourceLocation CaptureDefaultLoc, bool ExplicitParams,
>> +             bool ExplicitResultType, ArrayRef<Expr *> CaptureInits,
>> +             SourceLocation ClosingBrace, bool
>> ContainsUnexpandedParameterPack);
>>
>>    /// Construct an empty lambda expression.
>>    LambdaExpr(EmptyShell Empty, unsigned NumCaptures)
>> @@ -1888,9 +1887,9 @@ class LambdaExpr final : public Expr,
>>    static LambdaExpr *
>>    Create(const ASTContext &C, CXXRecordDecl *Class, SourceRange
>> IntroducerRange,
>>           LambdaCaptureDefault CaptureDefault, SourceLocation
>> CaptureDefaultLoc,
>> -         ArrayRef<LambdaCapture> Captures, bool ExplicitParams,
>> -         bool ExplicitResultType, ArrayRef<Expr *> CaptureInits,
>> -         SourceLocation ClosingBrace, bool
>> ContainsUnexpandedParameterPack);
>> +         bool ExplicitParams, bool ExplicitResultType,
>> +         ArrayRef<Expr *> CaptureInits, SourceLocation ClosingBrace,
>> +         bool ContainsUnexpandedParameterPack);
>>
>>    /// Construct a new lambda expression that will be deserialized from
>>    /// an external source.
>>
>> diff  --git a/clang/lib/AST/ASTImporter.cpp
>> b/clang/lib/AST/ASTImporter.cpp
>> index 10035162299e..a2a712e6b6ca 100644
>> --- a/clang/lib/AST/ASTImporter.cpp
>> +++ b/clang/lib/AST/ASTImporter.cpp
>> @@ -1890,6 +1890,19 @@ Error ASTNodeImporter::ImportDefinition(
>>          // set in CXXRecordDecl::CreateLambda.  We must import the
>> contained
>>          // decls here and finish the definition.
>>          (To->isLambda() && shouldForceImportDeclContext(Kind))) {
>> +      if (To->isLambda()) {
>> +        auto *FromCXXRD = cast<CXXRecordDecl>(From);
>> +        SmallVector<LambdaCapture, 8> ToCaptures;
>> +        ToCaptures.reserve(FromCXXRD->capture_size());
>> +        for (const auto &FromCapture : FromCXXRD->captures()) {
>> +          if (auto ToCaptureOrErr = import(FromCapture))
>> +            ToCaptures.push_back(*ToCaptureOrErr);
>> +          else
>> +            return ToCaptureOrErr.takeError();
>> +        }
>> +        cast<CXXRecordDecl>(To)->setCaptures(ToCaptures);
>> +      }
>> +
>>        Error Result = ImportDeclContext(From, /*ForceImport=*/true);
>>        // Finish the definition of the lambda, set isBeingDefined to
>> false.
>>        if (To->isLambda())
>> @@ -7588,15 +7601,6 @@ ExpectedStmt
>> ASTNodeImporter::VisitLambdaExpr(LambdaExpr *E) {
>>    if (!ToCallOpOrErr)
>>      return ToCallOpOrErr.takeError();
>>
>> -  SmallVector<LambdaCapture, 8> ToCaptures;
>> -  ToCaptures.reserve(E->capture_size());
>> -  for (const auto &FromCapture : E->captures()) {
>> -    if (auto ToCaptureOrErr = import(FromCapture))
>> -      ToCaptures.push_back(*ToCaptureOrErr);
>> -    else
>> -      return ToCaptureOrErr.takeError();
>> -  }
>> -
>>    SmallVector<Expr *, 8> ToCaptureInits(E->capture_size());
>>    if (Error Err = ImportContainerChecked(E->capture_inits(),
>> ToCaptureInits))
>>      return std::move(Err);
>> @@ -7608,11 +7612,11 @@ ExpectedStmt
>> ASTNodeImporter::VisitLambdaExpr(LambdaExpr *E) {
>>    if (Err)
>>      return std::move(Err);
>>
>> -  return LambdaExpr::Create(
>> -      Importer.getToContext(), ToClass, ToIntroducerRange,
>> -      E->getCaptureDefault(), ToCaptureDefaultLoc, ToCaptures,
>> -      E->hasExplicitParameters(), E->hasExplicitResultType(),
>> ToCaptureInits,
>> -      ToEndLoc, E->containsUnexpandedParameterPack());
>> +  return LambdaExpr::Create(Importer.getToContext(), ToClass,
>> ToIntroducerRange,
>> +                            E->getCaptureDefault(), ToCaptureDefaultLoc,
>> +                            E->hasExplicitParameters(),
>> +                            E->hasExplicitResultType(), ToCaptureInits,
>> +                            ToEndLoc,
>> E->containsUnexpandedParameterPack());
>>  }
>>
>>
>>
>> diff  --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
>> index 5412aa7ed5e0..4d184b5a4703 100644
>> --- a/clang/lib/AST/DeclCXX.cpp
>> +++ b/clang/lib/AST/DeclCXX.cpp
>> @@ -664,8 +664,7 @@ bool
>> CXXRecordDecl::lambdaIsDefaultConstructibleAndAssignable() const {
>>    // C++17 [expr.prim.lambda]p21:
>>    //   The closure type associated with a lambda-expression has no
>> default
>>    //   constructor and a deleted copy assignment operator.
>> -  if (getLambdaCaptureDefault() != LCD_None ||
>> -      getLambdaData().NumCaptures != 0)
>> +  if (getLambdaCaptureDefault() != LCD_None || capture_size() != 0)
>>      return false;
>>    return getASTContext().getLangOpts().CPlusPlus20;
>>  }
>> @@ -1367,6 +1366,24 @@ void
>> CXXRecordDecl::finishedDefaultedOrDeletedMember(CXXMethodDecl *D) {
>>      data().DeclaredNonTrivialSpecialMembers |= SMKind;
>>  }
>>
>> +void CXXRecordDecl::setCaptures(ArrayRef<LambdaCapture> Captures) {
>> +  ASTContext &Context = getASTContext();
>> +  CXXRecordDecl::LambdaDefinitionData &Data = getLambdaData();
>> +
>> +  // Copy captures.
>> +  Data.NumCaptures = Captures.size();
>> +  Data.NumExplicitCaptures = 0;
>> +  Data.Captures = (LambdaCapture
>> *)Context.Allocate(sizeof(LambdaCapture) *
>> +                                                    Captures.size());
>> +  LambdaCapture *ToCapture = Data.Captures;
>> +  for (unsigned I = 0, N = Captures.size(); I != N; ++I) {
>> +    if (Captures[I].isExplicit())
>> +      ++Data.NumExplicitCaptures;
>> +
>> +    *ToCapture++ = Captures[I];
>> +  }
>> +}
>> +
>>  void CXXRecordDecl::setTrivialForCallFlags(CXXMethodDecl *D) {
>>    unsigned SMKind = 0;
>>
>>
>> diff  --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
>> index cfbf2c88c4cc..2b148b4fa768 100644
>> --- a/clang/lib/AST/ExprCXX.cpp
>> +++ b/clang/lib/AST/ExprCXX.cpp
>> @@ -1087,35 +1087,18 @@ LambdaCaptureKind LambdaCapture::getCaptureKind()
>> const {
>>
>>  LambdaExpr::LambdaExpr(QualType T, SourceRange IntroducerRange,
>>                         LambdaCaptureDefault CaptureDefault,
>> -                       SourceLocation CaptureDefaultLoc,
>> -                       ArrayRef<LambdaCapture> Captures, bool
>> ExplicitParams,
>> +                       SourceLocation CaptureDefaultLoc, bool
>> ExplicitParams,
>>                         bool ExplicitResultType, ArrayRef<Expr *>
>> CaptureInits,
>>                         SourceLocation ClosingBrace,
>>                         bool ContainsUnexpandedParameterPack)
>>      : Expr(LambdaExprClass, T, VK_RValue, OK_Ordinary),
>>        IntroducerRange(IntroducerRange),
>> CaptureDefaultLoc(CaptureDefaultLoc),
>> -      NumCaptures(Captures.size()), CaptureDefault(CaptureDefault),
>> +      NumCaptures(CaptureInits.size()), CaptureDefault(CaptureDefault),
>>        ExplicitParams(ExplicitParams),
>> ExplicitResultType(ExplicitResultType),
>>        ClosingBrace(ClosingBrace) {
>> -  assert(CaptureInits.size() == Captures.size() && "Wrong number of
>> arguments");
>>    CXXRecordDecl *Class = getLambdaClass();
>> -  CXXRecordDecl::LambdaDefinitionData &Data = Class->getLambdaData();
>> -
>> -  // FIXME: Propagate "has unexpanded parameter pack" bit.
>> -
>> -  // Copy captures.
>> -  const ASTContext &Context = Class->getASTContext();
>> -  Data.NumCaptures = NumCaptures;
>> -  Data.NumExplicitCaptures = 0;
>> -  Data.Captures =
>> -      (LambdaCapture *)Context.Allocate(sizeof(LambdaCapture) *
>> NumCaptures);
>> -  LambdaCapture *ToCapture = Data.Captures;
>> -  for (unsigned I = 0, N = Captures.size(); I != N; ++I) {
>> -    if (Captures[I].isExplicit())
>> -      ++Data.NumExplicitCaptures;
>> -
>> -    *ToCapture++ = Captures[I];
>> -  }
>> +  assert(NumCaptures == Class->capture_size() && "Wrong number of
>> captures");
>> +  assert(CaptureDefault == Class->getLambdaCaptureDefault());
>>
>>    // Copy initialization expressions for the non-static data members.
>>    Stmt **Stored = getStoredStmts();
>> @@ -1128,22 +1111,24 @@ LambdaExpr::LambdaExpr(QualType T, SourceRange
>> IntroducerRange,
>>    setDependence(computeDependence(this,
>> ContainsUnexpandedParameterPack));
>>  }
>>
>> -LambdaExpr *LambdaExpr::Create(
>> -    const ASTContext &Context, CXXRecordDecl *Class,
>> -    SourceRange IntroducerRange, LambdaCaptureDefault CaptureDefault,
>> -    SourceLocation CaptureDefaultLoc, ArrayRef<LambdaCapture> Captures,
>> -    bool ExplicitParams, bool ExplicitResultType, ArrayRef<Expr *>
>> CaptureInits,
>> -    SourceLocation ClosingBrace, bool ContainsUnexpandedParameterPack) {
>> +LambdaExpr *LambdaExpr::Create(const ASTContext &Context, CXXRecordDecl
>> *Class,
>> +                               SourceRange IntroducerRange,
>> +                               LambdaCaptureDefault CaptureDefault,
>> +                               SourceLocation CaptureDefaultLoc,
>> +                               bool ExplicitParams, bool
>> ExplicitResultType,
>> +                               ArrayRef<Expr *> CaptureInits,
>> +                               SourceLocation ClosingBrace,
>> +                               bool ContainsUnexpandedParameterPack) {
>>    // Determine the type of the expression (i.e., the type of the
>>    // function object we're creating).
>>    QualType T = Context.getTypeDeclType(Class);
>>
>> -  unsigned Size = totalSizeToAlloc<Stmt *>(Captures.size() + 1);
>> +  unsigned Size = totalSizeToAlloc<Stmt *>(CaptureInits.size() + 1);
>>    void *Mem = Context.Allocate(Size);
>>    return new (Mem)
>>        LambdaExpr(T, IntroducerRange, CaptureDefault, CaptureDefaultLoc,
>> -                 Captures, ExplicitParams, ExplicitResultType,
>> CaptureInits,
>> -                 ClosingBrace, ContainsUnexpandedParameterPack);
>> +                 ExplicitParams, ExplicitResultType, CaptureInits,
>> ClosingBrace,
>> +                 ContainsUnexpandedParameterPack);
>>  }
>>
>>  LambdaExpr *LambdaExpr::CreateDeserialized(const ASTContext &C,
>>
>> diff  --git a/clang/lib/Sema/SemaLambda.cpp
>> b/clang/lib/Sema/SemaLambda.cpp
>> index b4336aa430eb..e751a73957d0 100644
>> --- a/clang/lib/Sema/SemaLambda.cpp
>> +++ b/clang/lib/Sema/SemaLambda.cpp
>> @@ -1782,6 +1782,8 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation
>> StartLoc, SourceLocation EndLoc,
>>        CaptureInits.push_back(Init.get());
>>      }
>>
>> +    Class->setCaptures(Captures);
>> +
>>      // C++11 [expr.prim.lambda]p6:
>>      //   The closure type for a lambda-expression with no lambda-capture
>>      //   has a public non-virtual non-explicit const conversion function
>> @@ -1811,7 +1813,6 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation
>> StartLoc, SourceLocation EndLoc,
>>
>>    LambdaExpr *Lambda = LambdaExpr::Create(Context, Class,
>> IntroducerRange,
>>                                            CaptureDefault,
>> CaptureDefaultLoc,
>> -                                          Captures,
>>                                            ExplicitParams,
>> ExplicitResultType,
>>                                            CaptureInits, EndLoc,
>>
>>  ContainsUnexpandedParameterPack);
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200605/c3743cb7/attachment-0001.html>


More information about the lldb-commits mailing list