[all-commits] [llvm/llvm-project] 22447a: [lldb] Mark the implicit copy constructor as delet...

[Raphael Isemann via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 22447a61d405a9e279c7dad72b342dcc6e8b1b4b
      https://github.com/llvm/llvm-project/commit/22447a61d405a9e279c7dad72b342dcc6e8b1b4b
  Author: Raphael Isemann <teemperor at gmail.com>
  Date:   2020-01-20 (Mon, 20 Jan 2020)

  Changed paths:
    R lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
    R lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
    A lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/TestDeletingImplicitCopyConstructor.py
    A lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp
    M lldb/source/Symbol/ClangASTContext.cpp
    M lldb/unittests/Symbol/TestClangASTContext.cpp

  Log Message:
  -----------
  [lldb] Mark the implicit copy constructor as deleted when a move constructor is provided.

Summary:
CXXRecordDecls that have a move constructor but no copy constructor need to
have their implicit copy constructor marked as deleted (see C++11 [class.copy]p7, p18)
Currently we don't do that when building an AST with ClangASTContext which causes
Sema to realise that the AST is malformed and asserting when trying to create an implicit
copy constructor for us in the expression:
```
Assertion failed: ((data().DefaultedCopyConstructorIsDeleted || needsOverloadResolutionForCopyConstructor())
    && "Copy constructor should not be deleted"), function setImplicitCopyConstructorIsDeleted, file include/clang/AST/DeclCXX.h, line 828.
```

In the test case there is a class `NoCopyCstr` that should have its copy constructor marked as
deleted (as it has a move constructor). When we end up trying to tab complete in the
`IndirectlyDeletedCopyCstr` constructor, Sema realises that the `IndirectlyDeletedCopyCstr`
has no implicit copy constructor and tries to create one for us. It then realises that
`NoCopyCstr` also has no copy constructor it could find via lookup. However because we
haven't marked the FieldDecl as having a deleted copy constructor the
`needsOverloadResolutionForCopyConstructor()` returns false and the assert fails.
`needsOverloadResolutionForCopyConstructor()` would return true if during the time we
added the `NoCopyCstr` FieldDecl to `IndirectlyDeletedCopyCstr` we would have actually marked
it as having a deleted copy constructor (which would then mark the copy constructor of
`IndirectlyDeletedCopyCstr ` as needing overload resolution and Sema is happy).

This patch sets the correct mark when we complete our CXXRecordDecls (which is the time when
we know whether a copy constructor has been declared). In theory we don't have to do this if
we had a Sema around when building our debug info AST but at the moment we don't have this
so this has to do the job for now.

Reviewers: shafik

Reviewed By: shafik

Subscribers: aprantl, JDevlieghere, lldb-commits

Tags: #lldb

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