[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In D50119#1308869 , @rjmccall wrote: > Hmm. I don't remember what we do in this case for `trivial_abi` (which does > need to address it because it's possible to return types like `DestroyBase`). > It looks like we currently

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added a comment. In D50119#1305503 , @Quuxplusone wrote: > In D50119#1303720 , @rjmccall wrote: > > > In D50119#1303662 , @Quuxplusone > > wrote: > > > > > >> I sti

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In https://reviews.llvm.org/D50119#1303720, @rjmccall wrote: > In https://reviews.llvm.org/D50119#1303662, @Quuxplusone wrote: > > > >> I still believe it is impossible to implement `std::optional` with only > > >> `[[maybe_trivially_relocatable]]`. > > > > > > This

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added a comment. In https://reviews.llvm.org/D50119#1303662, @Quuxplusone wrote: > In https://reviews.llvm.org/D50119#1303577, @rjmccall wrote: > > > In https://reviews.llvm.org/D50119#1303423, @Quuxplusone wrote: > > > > > In the `unordered_set [[maybe_trivially_relocatable]]` patch, I

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In https://reviews.llvm.org/D50119#1303577, @rjmccall wrote: > In https://reviews.llvm.org/D50119#1303423, @Quuxplusone wrote: > > > In the `unordered_set [[maybe_trivially_relocatable]]` patch, I must wrap > > the attribute in a macro > > `_LIBCPP_MAYBE_TRIVIALLY_R

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added a comment. In https://reviews.llvm.org/D50119#1303423, @Quuxplusone wrote: > In the `unordered_set [[maybe_trivially_relocatable]]` patch, I must wrap the > attribute in a macro `_LIBCPP_MAYBE_TRIVIALLY_RELOCATABLE_UNLESS_DEBUG`. > Without this caveat, I would have ended up with

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In https://reviews.llvm.org/D50119#1302134, @Quuxplusone wrote: > Now that `[[clang::maybe_trivially_relocatable]]` is implemented, we can see > if it does actually simplify the libc++ patch. I will try to get to that > tonight. Here is `deque` implemented with `[

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 174513. Quuxplusone added a comment. Implement `[[clang::maybe_trivially_relocatable]]` along the lines suggested by @rjmccall. The idea is that there are two levels of "opt-in-ness." The first level, `[[clang::maybe_trivially_relocatable]]`, means "I wa

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:3811 case ParsedAttr::AT_CXX11NoReturn: + case ParsedAttr::AT_TriviallyRelocatable: return true; erichkeane wrote: > A note for future reviewers, after the C++11 spelling is remove

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added a reviewer: ahatanak. rjmccall added inline comments. Comment at: include/clang/AST/DeclCXX.h:482 +/// and a defaulted destructor. +unsigned IsNaturallyTriviallyRelocatable : 1; + Quuxplusone wrote: > erichkeane wrote: > > Quuxplusone wrote

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: include/clang/AST/DeclCXX.h:482 +/// and a defaulted destructor. +unsigned IsNaturallyTriviallyRelocatable : 1; + erichkeane wrote: > Quuxplusone wrote: > > erichkeane wrote: > > > Typically we'd have this ca

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 174498. Quuxplusone marked 14 inline comments as done. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Use `[[clang::trivially_relocatable]]` instead of `[[trivially_relocatable]]`. Canonicalize in `QualType::isTriviallyReloca

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) Quuxplusone wrote: > dblaikie wrote: > > Quuxplus

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

dblaikie added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) Quuxplusone wrote: > dblaikie wrote: > > Quuxplus

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) dblaikie wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

dblaikie added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) Quuxplusone wrote: > rjmccall wrote: > > Quuxplus

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2096 +def TriviallyRelocatable : InheritableAttr { + let Spellings = [CXX11<"", "trivially_relocatable", 200809>, + CXX11<"clang", "trivially_relocatable">]; erichkean

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) Quuxplusone wrote: > rjmccall wrote: > > Quuxplus

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) Quuxplusone wrote: > rjmccall wrote: > > Quuxplus

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) Quuxplusone wrote: > rjmccall wrote: > > Quuxplus

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

erichkeane added inline comments. Comment at: lib/AST/Type.cpp:2234 +bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const { + QualType T = Context.getBaseElementType(*this); + if (T->isIncompleteType()) Quuxplusone wrote: > erichkeane wrot

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > @rjmc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) Quuxplusone wrote: > @rjmccall wrote: > > trivial

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone marked 3 inline comments as done. Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013)

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

rjmccall added a comment. In https://reviews.llvm.org/D50119#1297070, @dblaikie wrote: > Hey @rjmccall - I'm trying to remember, but can't get it quite clear in my > head. I seem to recall some discussion about trivial_abi not implying/being > strong enough for all the cases that trivial_reloca

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

erichkeane added inline comments. Comment at: docs/LanguageExtensions.rst:1093 library. +* ``__is_trivially_relocatable`` (Clang): Determines whether moving an object + of type ``type``, and then destroying the source object, is functionally Quuxplusone wrote

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

dblaikie added a comment. Hey @rjmccall - I'm trying to remember, but can't get it quite clear in my head. I seem to recall some discussion about trivial_abi not implying/being strong enough for all the cases that trivial_relocatable sounds like it covers? Do you happen to remember the distinct

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1093 library. +* ``__is_trivially_relocatable`` (Clang): Determines whether moving an object + of type ``type``, and then destroying the source object, is

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

erichkeane added subscribers: aaron.ballman, erichkeane. erichkeane added a comment. You'll not be able to get the C++ spelling without it getting into the standard. Additionally, it seems easy enough to calculate upon need, so storing it in the AST is a waste. @aaron.ballman is likely another

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 161866. Quuxplusone added a comment. Update to match the wording in D1144R0 draft revision 13. There's no longer any need to fail when we see a mutable member; on the other hand, we *always* need to perform the special member lookup to find out if our cl

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 161190. Quuxplusone added a comment. A type with no move-constructor, and some non-defaulted copy-constructors (even if it *also* has some defaulted copy-constructors) should not be considered naturally trivially relocatable. The paper already said this,

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Rakete added a comment. It would be nice to be able to diagnose `X`: template struct Foo { struct [[trivially_relocatable]] X { // no warning X(X &&) = delete; }; }; Foo f; // no warning static_assert(!__is_trivially_relocatable(Foo::X)); // ok But otherwise, you

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 160642. Quuxplusone marked an inline comment as done. Quuxplusone added a comment. Rebased, and ping! Repository: rC Clang https://reviews.llvm.org/D50119 Files: docs/LanguageExtensions.rst include/clang/AST/DeclCXX.h include/clang/AST/Type.h

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6187 +Record->dropAttr(); + } else if (Record->needsImplicitMoveConstructor() && + Record->defaultedMoveConstructorIsDeleted()) { Rakete wrote: > Rakete w

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 159073. Quuxplusone marked 8 inline comments as done. Quuxplusone added a comment. Correctly drop the attribute from non-relocatable instantiations of class templates (even though they don't print the diagnostic). Repository: rC Clang https://reviews

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6091 + for (auto *F : Record->fields()) { +if (F->isMutable()) { Rakete wrote: > Can you move this in `ActOnFields`? That way we avoid

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Rakete added a reviewer: Rakete. Rakete added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6091 + for (auto *F : Record->fields()) { +if (F->isMutable()) { Quuxplusone wrote: > Rakete wrote: > > Can you move this in `ActOnFields`?

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Rakete added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6174 + Record->hasAttr() && + !isTemplateInstantiation(Record->getTemplateSpecializationKind())) { +if (Record->getDefinition() && !Record->isDependentContext() && The call to `i

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Rakete added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6187 +Record->dropAttr(); + } else if (Record->needsImplicitMoveConstructor() && + Record->defaultedMoveConstructorIsDeleted()) { Rakete wrote: > Quuxplusone w

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Rakete added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6187 +Record->dropAttr(); + } else if (Record->needsImplicitMoveConstructor() && + Record->defaultedMoveConstructorIsDeleted()) { Quuxplusone wrote: > Rakete w

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

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 Rakete wrote: > What's this file? A mistake? Yeah, it's p

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 158822. Quuxplusone marked 20 inline comments as done. Quuxplusone added a comment. Further removal of dead code based on @Rakete's feedback. Repository: rC Clang https://reviews.llvm.org/D50119 Files: docs/LanguageExtensions.rst include/clan

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Rakete 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 What's this file? A mistake? Comment at: in

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 158615. Quuxplusone added a comment. clang-format. Repository: rC Clang https://reviews.llvm.org/D50119 Files: compiler-explorer-llvm-commit.sh docs/LanguageExtensions.rst include/clang/AST/DeclCXX.h include/clang/Basic/Attr.td include/clan

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2942 + "not %select{move-constructible|destructible}2" + >; + > Might be useful to add a note explaining why the type isn't trivially > relocatable instead of the general

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 158613. Quuxplusone added a comment. Address feedback from @Rakete. Fix wrong ordering of 'class|struct|...' in the diagnostic. Add `union` test cases. Correctly drop the attribute whenever it is diagnosed as inapplicable. Repository: rC Clang h

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Rakete added a comment. - There's a bug in your implementation: struct X { X &operator=(X &&); }; static_assert(__is_trivially_relocatable(X)); // oops, fires! `X` has a move constructor and a destructor, so it is trivially relocatable. - Please run your code through clang-format.

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone created this revision. Quuxplusone added a reviewer: rsmith. Quuxplusone added a project: clang. Herald added a subscriber: cfe-commits. This is the compiler half of C++ proposal 1144 "Object relocation in terms of move plus destroy," as seen on https://godbolt.org/g/zUUAVW and https