[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 9 20:06:42 PDT 2018


rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1725
                                BlockFieldFlags Flags, bool EHOnly,
+                               bool DisposeCannotThrow, VarDecl *Var,
                                CodeGenFunction &CGF) {
----------------
ahatanak wrote:
> rjmccall wrote:
> > Could you replace these two flags with something more semantic, like telling this function what the context of pushing the cleanup is — basically meaning, are we in the copy helper or the destroy helper?  That will let you pull the comment explaining `DisposeCannotThrow` into this function, where it makes a lot more sense.
> I thought about passing a single flag instead of passing two flags too. If we are going to pass a single flag, should we still use two variables inside the function, EHOnly and DisposeCannotThrow, to maintain the readability of the code?
Computing `EHOnly` at the top makes sense to me.  `DisposeCannotThrow` is really only interesting in the one case, and the analysis makes more sense in terms of a `ForCopyHelper` parameter than it would in terms of this more abstract concept.


Repository:
  rC Clang

https://reviews.llvm.org/D50152





More information about the cfe-commits mailing list