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