aaron.ballman added inline comments.

================
Comment at: clang/test/AST/Interp/records.cpp:317-318
     {
-      auto T = Test(Arr, Pos);
+      Test(Arr, Pos);
       // End of scope, should destroy Test.
     }
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > Nit: nothing actually tests that this object is destroyed correctly. 
> > > > Here's an interesting test to consider:
> > > > ```
> > > > struct S {
> > > >   constexpr S() {}
> > > >   constexpr ~S() noexcept(false) { throw 12; }
> > > > };
> > > > 
> > > > constexpr int f() {
> > > >   S{};
> > > >   return 12;
> > > > }
> > > > 
> > > > static_assert(f() == 12);
> > > > ```
> > > > That should fail because `~S()` would hit the `throw` expression and 
> > > > thus is not valid. Note, you'll need to add `-Wno-invalid-constexpr` to 
> > > > your test to avoid the warning-defaults-to-error about the destructor 
> > > > never producing a constant expression.
> > > There are multiple reasons why that sample is not rejected right now, one 
> > > I can easily fix in a follow-up patch, the other one would actually 
> > > require us to recognize the `throw` and reject it with a proper 
> > > diagnostic.
> > We should definitely fix the `throw` at some point, but any of the 
> > dynamically reachable problematic constructs would work (`dynamic_cast` 
> > whose type would throw, invocation of the `va_arg` macro, 
> > `reinterpret_cast`, etc)
> Yes, I think we need a new opcode for that so we only emit the diagnostic 
> when such a construct is actually executed.
Oh yeah, you'll definitely need that, a whole pile of the constexpr rules are 
based around code reachability.

Are you saying you've got no way to test this until you implement that opcode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147591

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

Reply via email to