Rakete1111 added inline comments.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:6187
+        Record->dropAttr<TriviallyRelocatableAttr>();
+      } else if (Record->needsImplicitMoveConstructor() &&
+                 Record->defaultedMoveConstructorIsDeleted()) {
----------------
Rakete1111 wrote:
> Quuxplusone wrote:
> > Rakete1111 wrote:
> > > This is dead code. `Record` never needs an implicit move constructor at 
> > > this point, because either 1) it never did or 2) it was defined above by 
> > > `LookupSpecialMember`.
> > Confirmed that this code is never hit; and removed. Just for my own 
> > information: you're saying that the call to `LookupSpecialMember` on line 
> > 6179, even though it's looking up the //destructor//, will actually cause 
> > all the `needsImplicitFootor` flags to get resolved? And so also I guess I 
> > should never have been looking at those flags directly; I should have 
> > handled this case by calling `LookupSpecialMember` like I do on line 6196. 
> > Is that basically correct?
> No, not the 6179 one, but the one before it 6163. Yeah you could have :)
Sorry for the noise, that isn't it because of the if statement right before 
6163 :/. I was wrong...

Every implicit constructor is already defined before the call to 
`CheckCompletedCXXClass` (except if it's a template), so `needsImplicitFootor` 
is always `false`. This means that you can drop the if statement right before 
6163, because it's always true.

I'm 99% sure of the previous paragraph. :)


================
Comment at: lib/Sema/SemaDeclCXX.cpp:6091
 
+  for (auto *F : Record->fields()) {
+    if (F->isMutable()) {
----------------
Can you move this in `ActOnFields`? That way we avoid two iterations of the 
fields.


Repository:
  rC Clang

https://reviews.llvm.org/D50119



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to