ahatanak added inline comments.
================
Comment at: include/clang/AST/ASTContext.h:158
+ Expr *CopyExpr;
+ bool CanThrow;
+ };
----------------
rjmccall wrote:
> Using a PointerIntPair in the implementation of this is still a reasonable
> choice. Just make it a proper abstraction, with a constructor with the two
> arguments and getters for the two fields.
I also needed a function that sets the pointer and flag.
================
Comment at: lib/CodeGen/CGBlocks.cpp:1682
+ if (IsCopyHelper && Ctx.getBlockVarCopyInits(Var).CanThrow)
+ Name += "c";
+ }
----------------
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?
================
Comment at: lib/CodeGen/CGBlocks.cpp:1688
+ if (F == BLOCK_FIELD_IS_BLOCK)
+ Name += "b";
+ }
----------------
rjmccall wrote:
> Why `rb` for a captured block instead of some single-letter thing? You don't
> need to emulate the structure of the flags here.
I can use a single letter here, but I'm already using 'b' for byref captures.
Perhaps I can use 'o' for non-arc objects, instead of 'r', and use 'r' for
byref?
================
Comment at: lib/CodeGen/CGBlocks.cpp:1705
+ IsVolatile, Ctx);
+ Name += llvm::to_string(Str.size()) + "_" + Str;
+ break;
----------------
rjmccall wrote:
> The underscore is necessary here because non-trivial destructor strings can
> start with a number? Worth a comment.
Yes, that's correct.
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
