rsmith added inline comments.
================
Comment at: lib/Analysis/ThreadSafety.cpp:1994
+static CXXConstructorDecl *findConstructorForByValueReturn(CXXRecordDecl *RD) {
+ // Prefer a move constructor over a copy constructor. If there's more than
----------------
aaron.ballman wrote:
> Can you sprinkle some const correctness around this declaration?
I can't make the `CXXConstructorDecl` const without adding a `const_cast`
somewhere or making very large-scale changes to Clang :) The `CXXConstructExpr`
constructor needs a pointer to a non-const `CXXConstructorDecl`.
I can make the `CXXRecordDecl` const. I think that's actually a
const-correctness bug -- if `const` on an AST node means anything, it should
mean that the AST is potentially immutable, and you shouldn't be able to get
from a const AST node to a non-const one -- but it's probably closer to the
ideal than no const, so I'll add a const there.
================
Comment at: lib/Analysis/ThreadSafety.cpp:2001
+ for (CXXConstructorDecl *Ctor : RD->ctors()) {
+ if (Ctor->isDeleted() || Ctor->getAccess() != AS_public)
+ continue;
----------------
aaron.ballman wrote:
> Should we care about access checking here (protected ctor within a reasonable
> context, or friendship)?
I'm not completely sure. My thinking was that if someone uses the "private copy
constructor with no definition" approach rather than the "deleted copy
constructor" approach, we should ignore that copy constructor. But this would
only really matter if they do that to their *move* constructor and they have an
accessible copy constructor, which seems like a sufficiently bizarre situation
to not have a special rule for it. (Well, it'd also matter if the private copy
ctor with no definition had thread safety attributes, which also seems like a
very strange case.)
Happy to go either way here. On balance I think I agree that it's more
consistent with the general language rules to ignore access here.
Repository:
rC Clang
https://reviews.llvm.org/D41933
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
