ahatanak added inline comments.

================
Comment at: lib/CodeGen/CGBlocks.cpp:1682
+          if (IsCopyHelper && Ctx.getBlockVarCopyInits(Var).CanThrow)
+            Name += "c";
+        }
----------------
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > I don't think you need to add `d` to the name of a copy helper.  It's a 
> > > bit weird, but while copying a `__block` variable can cause its copy 
> > > helper to run, destroying it immediately afterwards can never cause its 
> > > destroy helper to run.  That's because a newly-copied `__block` variable 
> > > always has a reference count of 2: the new reference in the copy and the 
> > > forwarding reference from the original.
> > > 
> > > I think that means you can just add a single letter which specifies 
> > > whether the corresponding `__block` variable operation is known to be 
> > > able to throw.
> > I added 'd' to the name of the copy helper functions only because IRGen 
> > generates different code depending on whether the destructor can throw or 
> > not.
> > 
> > For example, if I compile the following code with -DTHROWS, IRGen uses 
> > 'invoke' (which jumps to the terminate block) for the calls to 
> > `_Block_object_dispose` on the EH path whereas it uses 'call' if the 
> > destructor doesn't throw.
> > 
> > ```
> > struct S {
> >   S();
> > #ifdef THROWS
> >   ~S() noexcept(false);
> > #else
> >   ~S() noexcept(true);
> > #endif
> >   S(const S &);
> >   int a;
> > };
> > 
> > void test() {
> >   __block S s0, s1, s2;
> >   ^{ (void)s0, (void)s1; (void)s2; };
> > }
> > ```
> > 
> > It seems like IRGen doesn't have to use 'invoke' when emitting a call to  
> > `_Block_object_dispose` even when the class has a destructor that can 
> > throw, if I understood your explanation correctly?
> Right.  It's specifically only true when unwinding after a copy, which is 
> very atypical for C++ code, but nonetheless it's true.  We should make the 
> call `nounwind` in these situations and leave a comment explaining why.  Did 
> my explanation make any sense?
Yes, it makes sense. Since the reference count is two after the `__block` 
variable is copied, calling `_Block_object_dispose` on it won't cause the 
destructor to be called.


Repository:
  rC Clang

https://reviews.llvm.org/D50152



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

Reply via email to