rsmith added inline comments.
================
Comment at: lib/CodeGen/CGCXXABI.cpp:33-47
+ // See also Sema::ShouldDeleteSpecialMember. These two functions
+ // should be kept consistent.
+
// If RD has a non-trivial move or copy constructor, we cannot copy the
// argument.
if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor())
return false;
----------------
Perhaps it would make sense to put this whole "does the language permit passing
in registers?" calculation in Sema and store that in the DefinitionData rather
than storing the "HasNonDeletedCopyOrMoveConstructor" flag.
================
Comment at: lib/Sema/SemaDecl.cpp:14816-14817
+static bool HasNonDeletedCopyOrMoveCtor(CXXRecordDecl *CXXRD, Sema &SemaRef) {
+ // FIXME: Handle the cases in which CXXRD is dependent. We need to
instantiate
+ // the class to figure out if the constructor should be deleted.
+ assert(!CXXRD->isDependentType() && "Not implemented yet");
----------------
This seems like a good motivation for the flag to be a "can pass in registers"
flag: that makes it much more obvious that it's meaningless on a dependent
class, since such a class will never be passed / returned at all.
================
Comment at: lib/Sema/SemaDecl.cpp:14821-14822
+ CXXConstructorDecl *CopyCtor =
SemaRef.DeclareImplicitCopyConstructor(CXXRD);
+ if (SemaRef.ShouldDeleteSpecialMember(CopyCtor, Sema::CXXCopyConstructor))
+ SemaRef.SetDeclDeleted(CopyCtor, CXXRD->getLocation());
+ }
----------------
You don't need to do this: triggering the declaration will delete the member if
relevant.
However, we should refactor `ShouldDeleteSpecialMember` so that it can be
called without actually having built a special member declaration: this patch
removes all laziness in declaring copy or move constructors.
================
Comment at: lib/Sema/SemaDecl.cpp:14825
+
+ if (CXXRD->needsImplicitMoveConstructor()) {
+ CXXConstructorDecl *MoveCtor =
SemaRef.DeclareImplicitMoveConstructor(CXXRD);
----------------
This needs to be guarded by a `getLangOpts().CPlusPlus11` check.
================
Comment at: lib/Sema/SemaDecl.cpp:14831-14839
+ bool CopyOrMoveDeleted = false;
+ for (const CXXConstructorDecl *CD : CXXRD->ctors()) {
+ if (CD->isMoveConstructor() || CD->isCopyConstructor()) {
+ if (!CD->isDeleted()) {
+ return true;
+ }
+ CopyOrMoveDeleted = true;
----------------
We should do this check before we call `ShouldDeleteSpecialMember` to get the
benefit of the early exit. If we switch to tracking the ABI flag, we can also
bail out early if we see a non-trivial copy ctor / move ctor / dtor.
================
Comment at: lib/Sema/SemaDecl.cpp:14841-14852
+ // If a move constructor or move assignment operator was declared, the
+ // default copy constructors are implicitly deleted, except in one case
+ // related to compatibility with MSVC pre-2015.
+ if (CXXRD->hasUserDeclaredMoveConstructor())
+ CopyOrMoveDeleted = true;
+ if (CXXRD->hasUserDeclaredMoveAssignment()) {
+ const LangOptions &opts = SemaRef.getASTContext().getLangOpts();
----------------
Do we need this? The above code will have already declared as deleted the
relevant operator, so this seems like it can never trigger.
================
Comment at: lib/Sema/SemaDecl.cpp:15147-15148
if (!Completed)
Record->completeDefinition();
----------------
This call also needs to pass in the relevant flag.
https://reviews.llvm.org/D35056
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
