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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
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
----------------
erichkeane wrote:
> unrelated changes here?
Having whitespaces before newline is bad :/
This is editor doing so on file save,
and my git complains otherwise.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:560
+
+A function declared with ``__attribute__((const))`` attribute can not have
+infinite loops (i.e. they must either terminate or return to the caller,
----------------
erichkeane wrote:
> Similar changes to above.  This attribute is more of an 'assertion' by the developer that they promise they don't do these things (and we will UB otherwise), so I think they need to be written from that perspective.
> 
> I might suggest making an attempt to reword these docs.
> 
> ALSO, since these attributes can be spelled `clang::pure` and `clang::const` (IIRC?), I'd suggest we make the documents spelling-agnostic other than in direct code examples.
> I might suggest making an attempt to reword these docs.

This is my best attempt :)
If you can propose a better wording,
please feel free to do so.


================
Comment at: clang/lib/Analysis/CFG.cpp:2725
       NoReturn = true;
-    if (FD->hasAttr<NoThrowAttr>())
+    if (FD->hasAttr<NoUnwindAttr>() || FD->hasAttr<NoThrowAttr>())
       AddEHEdge = false;
----------------
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.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958