rjmccall accepted this revision.
rjmccall added a comment.

LGTM.



================
Comment at: lib/CodeGen/CGObjCGNU.cpp:915
+    return name;
+  }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a
----------------
theraven wrote:
> rjmccall wrote:
> > theraven wrote:
> > > rjmccall wrote:
> > > > Can this section-names cleanup also just be in a separate patch?
> > > This is difficult to extract, because it's mostly needed for the COFF 
> > > part where we need to modify the section names.  For ELF, it was fine to 
> > > keep them as separate `const char*`s
> > I don't mind if the extracted patch seems unmotivated on its own, but I do 
> > think it should be a separate patch.
> None of the changes to do this are in a separate commit, and they touch 
> enough of the code that it's very difficult to separate them without ending 
> up with conflicts in this branch (which touches the code surrounding the 
> changes in multiple places).  I cannot extract the code in such a way that 
> this patch would cleanly rebase on top, because various things (e.g. the null 
> section code) needed restructuring to work with Windows and touch this logic.
Alright, I won't insist.


================
Comment at: lib/CodeGen/CGObjCGNU.cpp:3817
+  if (isRethrow && CGF.CGM.getTarget().getTriple().isWindowsMSVCEnvironment()) 
{
+    CGF.EmitRuntimeCallOrInvoke(ExceptionReThrowFn).setDoesNotReturn();
+  }
----------------
theraven wrote:
> rjmccall wrote:
> > theraven wrote:
> > > rjmccall wrote:
> > > > You're sure here that the static information aligns with the dynamic?
> > > I'm not sure I understand this question.
> > Your new code path no longer uses `ExceptionAsObject`, which is our static 
> > notion of the current exception value.  Instead, you call a runtime 
> > function which presumably relies on a dynamically-stored current exception 
> > value.  I'm just asking if, in this lexical position, you're definitely 
> > rethrowing the right exception.
> Ah, I see.  This code path is hit only as a `@throw;`, not a `@throw obj` (in 
> the latter case, there is a non-null return from `S.getThrowExpr()`).  In 
> this case, the value of ExceptionAsObject may not be useful. In the case of a 
> catchall, this is a load from a stack location with no corresponding store.  
> The Windows unwind logic keeps the object on the stack during funclet 
> execution and then continues the unwind at the end, without ever providing 
> this frame's funclets with a pointer to it.  That's probably not very 
> obvious, so I've added an explanatory comment.
Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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

Reply via email to