Quuxplusone added inline comments.
================
Comment at: compiler-explorer-llvm-commit.sh:1
+# This is the commit of LLVM that we're currently based on.
+git reset --hard 1fa19f68007cd126a04448093c171f40e556087e
----------------
Rakete1111 wrote:
> What's this file? A mistake?
Yeah, it's pipework for the Compiler Explorer integration. I manually deleted
it from the previously uploaded diff, but forgot to delete it this time. You
can ignore it.
================
Comment at: include/clang/Sema/Sema.h:4304
+ bool IsTriviallyRelocatableType(QualType QT) const;
+
----------------
Rakete1111 wrote:
> Quuxplusone wrote:
> > Rakete1111 wrote:
> > > Any reason why this is a free function? Should be a member function of
> > > `QualType`.
> > `class QualType` is defined in `AST/`, whereas all the C++-specific
> > TriviallyRelocatable stuff is currently confined to `Sema/`. My impression
> > was that I should try to preserve that separation, even if it meant being
> > ugly right here. I agree that this is a stupid hack, and would love to get
> > rid of it, but I think I need some guidance as to how much mixing of `AST`
> > and `Sema` is permitted.
> Nah, it's fine. There are lots of C++ specific things in AST/, because the
> AST nodes represent C++-y stuff. Trivially copyable is also part of
> `QualType`, even though it's C++ specific.
Okay, moved!
================
Comment at: lib/Sema/SemaDeclAttr.cpp:6481
break;
+ case ParsedAttr::AT_TriviallyRelocatable:
+ handleTriviallyRelocatableAttr(S, D, AL);
----------------
Rakete1111 wrote:
> Why is this attribute under "Microsoft Attributes"? ;)
I dunno, the random `//comments` seem pretty silly to me. There's nothing
"Microsoft" about TrivialABI either (and no reason anyone should care, in this
context, that EmptyBases or LayoutVersion are "Microsoft"). I just put it here
because it's somewhat related to TrivialABI — and/or, to reduce code churn if
anyone ever decides to Do The Right Thing and alphabetize these switch cases.
================
Comment at: lib/Sema/SemaDeclCXX.cpp:6187
+ Record->dropAttr<TriviallyRelocatableAttr>();
+ } else if (Record->needsImplicitMoveConstructor() &&
+ Record->defaultedMoveConstructorIsDeleted()) {
----------------
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?
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
