rsmith added inline comments.
================
Comment at: include/clang/AST/DeclCXX.h:420
+ /// \brief True if this class has at least one non-deleted copy or move
+ /// constructor. That would allow passing it by registers.
----------------
rnk wrote:
> Isn't this "... at least one *trivial*, non-deleted copy or move
> constructor..."?
Changed to:
```
/// \brief True if this class can be passed in a non-address-preserving
/// fashion (such as in registers) according to the C++ language rules.
/// This does not imply anything about how the ABI in use will actually
/// pass an object of this class.
```
================
Comment at: include/clang/AST/DeclCXX.h:827
+ return data().DefaultedCopyConstructorIsDeleted;
+ }
+ /// \brief \c true if a defaulted move constructor for this class would be
----------------
v.g.vassilev wrote:
> Is there a reason for not keeping the default (for the file) 1 empty line
> between methods? Looks like if we add one new line before and after
> `hasSimpleMoveAssignment` is will be all consistent.
Done. (I was following the local style, but you're right that we don't do this
elsewhere in the class outside the `hasSimple` functions, excluding groups of
methods that are much more closely tied together such as `*_begin`/`*_end`.)
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:836
- // If this is true, the implicit copy constructor that Sema would have
- // created would not be deleted. FIXME: We should provide a more direct way
- // for CodeGen to ask whether the constructor was deleted.
- if (!RD->hasUserDeclaredCopyConstructor() &&
- !RD->hasUserDeclaredMoveConstructor() &&
- !RD->needsOverloadResolutionForMoveConstructor() &&
- !RD->hasUserDeclaredMoveAssignment() &&
- !RD->needsOverloadResolutionForMoveAssignment())
- return RAA_Default;
-
- // Otherwise, Sema should have created an implicit copy constructor if
- // needed.
- assert(!RD->needsImplicitCopyConstructor());
-
- // We have to make sure the trivial copy constructor isn't deleted.
- for (const CXXConstructorDecl *CD : RD->ctors()) {
- if (CD->isCopyConstructor()) {
- assert(CD->isTrivial());
- // We had at least one undeleted trivial copy ctor. Return directly.
- if (!CD->isDeleted())
- return RAA_Default;
+ // Win64 passes objects with non-deleted, non-trivial copy ctors
indirectly.
+ //
----------------
rnk wrote:
> This doesn't seem to match what we've computing, and it doesn't seem quite
> right. MSVC will pass a class with deleted, trivial copy ctors indirectly.
> Would it be correct to rephrase like this?
> "If RD has at least one trivial, non-deleted copy constructor, it is passed
> directly. Otherwise, it is passed indirectly."
You're right. I think "it is passed directly" is overspecifying, though, so how
about:
```
// If a class has at least one non-deleted, trivial copy constructor, it
// is passed according to the C ABI. Otherwise, it is passed indirectly.
```
================
Comment at: lib/Sema/SemaDeclCXX.cpp:5731
+/// registers, per C++ [class.temporary]p3.
+static bool computeCanPassInRegisters(Sema &S, CXXRecordDecl *D) {
+ if (D->isDependentType() || D->isInvalidDecl())
----------------
v.g.vassilev wrote:
> It would be very useful if we somehow assert if this function is called
> before the class triviality is computed?
Many of the functions we unconditionally call below will assert if the class
does not have a complete definition (eg, `needsImplicitCopyConstructor`).
================
Comment at: test/CodeGenCXX/uncopyable-args.cpp:101
+
+// In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC
2015, it is.
+// WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64
----------------
rnk wrote:
> Oh dear. :(
Can you check that MSVC 2013 is compatible with the code we produce here? (I've
checked 2015 passes this indirectly on Compiler Explorer.)
Repository:
rL LLVM
https://reviews.llvm.org/D35056
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
