[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I'll take a look at rewording the docs if no one else does.  I should hopefully have time next week, the rest of the patch is perhaps more important at the moment.



================
Comment at: clang/docs/ReleaseNotes.rst:841
   operator.
-- ``clang_Cursor_getNumTemplateArguments``, ``clang_Cursor_getTemplateArgumentKind``, 
-  ``clang_Cursor_getTemplateArgumentType``, ``clang_Cursor_getTemplateArgumentValue`` and 
+- ``clang_Cursor_getNumTemplateArguments``, ``clang_Cursor_getTemplateArgumentKind``,
+  ``clang_Cursor_getTemplateArgumentType``, ``clang_Cursor_getTemplateArgumentValue`` and
----------------
lebedev.ri wrote:
> erichkeane wrote:
> > unrelated changes here?
> Having whitespaces before newline is bad :/
> This is editor doing so on file save,
> and my git complains otherwise.
Yep, i had to turn that off in my editor at one point because I did this!  I just pushed d5fc931ba to remove that WS, so this will disappear in your  next rebase.


================
Comment at: clang/lib/Analysis/CFG.cpp:2725
       NoReturn = true;
-    if (FD->hasAttr<NoThrowAttr>())
+    if (FD->hasAttr<NoUnwindAttr>() || FD->hasAttr<NoThrowAttr>())
       AddEHEdge = false;
----------------
lebedev.ri wrote:
> erichkeane wrote:
> > I find myself thinking we should probably have a function in FunctionDecl that tests for the various states of the function, rather than keep checking the attribute's presence.  
> Yes, we have a huge spaghetti code spread through clang
> that tries to answer the same two question:
> 1. i'm a caller, if i call this function, might it throw?
> 1. i'm a callee, what should i do with exceptions that try to unwind out of me?
> 
> I don't know how to improve that, and i don't think just moving this into FunctionDecl would help.
> 
`FunctionDecl::WontThrow` and `FunctionDecl::ShouldUnwind` ? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138958/new/

https://reviews.llvm.org/D138958