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

Jonas Devlieghere via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 4 23:45:24 PDT 2020


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/

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?

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200604/db2e46a1/attachment-0001.html>


More information about the cfe-commits mailing list