royjacobson marked an inline comment as done.
royjacobson added inline comments.


================
Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK:             "kind": "ClassTemplateSpecializationDecl",
----------------
BRevzin wrote:
> royjacobson wrote:
> > BRevzin wrote:
> > > It's possible that I just don't understand what these tests actually mean 
> > > but... where is the test for `DefaultConstructorCheck<2>` having a 
> > > deleted default constructor, or `DefaultConstructorCheck<3>` having a 
> > > non-defaulted one?
> > > 
> > > It'd also be worthwhile to have at least one test with constaints that 
> > > subsume each other instead of being mutually exclusive. 
> > Should I check this? Except destructors, the other SMFs are found during 
> > overload resolution using the usual lookup that already takes into account 
> > delete/default/constraints etc.
> > 
> > This patch is about making sure that we set the triviality attributes 
> > correctly according to the eligible functions, so this is what I added 
> > tests for.
> > 
> > Most of this testing is done in the sema test, but I need this AST test as 
> > well to make sure we get the `canPassInRegisters` attribute correctly - 
> > we're doing some custom processing over the functions without the usual 
> > type attributes because there are some weird compatibility edge cases.
> > 
> One of the motivations for the paper is to ensure that like given:
> 
> ```
> template <class T>
> struct optional {
>     optional(optional const&) requires copyable<T> && trivially_copyableT> = 
> default;
>     optional(optional const&) requires copyable<T>;
> };
> ```
> 
> `optional<string>` is copyable (but not trivially copyable), `optional<int>` 
> is trivially copyable, and `optional<unique_ptr<int>>` isn't copyable. I'm 
> not sure what in here checks if that works. 
That's more-or-less the check in `constrained-special-member-functions.cpp:50`, 
I think.

Didn't acknowledge it in my first response - but yeah, I need some more 
complicated subsumption test cases



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128619/new/

https://reviews.llvm.org/D128619

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

Reply via email to