[PATCH] D94502: [FunctionAttrs] Derive willreturn for fns with readonly` & `mustprogress`.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:1436
+    F->addFnAttr(Attribute::WillReturn);
+    F->removeFnAttr(Attribute::NoReturn);
+  }
----------------
jdoerfert wrote:
> nikic wrote:
> > fhahn wrote:
> > > jdoerfert wrote:
> > > > `Changed = true;` ;)
> > > > Changed = true; ;)
> > >  
> > > yeah.
> > > 
> > > 
> > > 
> > > I guess a comment here might be helpful as well. It is possible that the function has been marked as `noreturn` earlier, e.g. because it does not have any returns. In such cases, `will return` seems a better choice, if possible.
> > Is it possible to keep both, or does that cause verifier errors? A function that always unwinds should be able to be both noreturn and willreturn without invoking UB.
> Good point. Both seem sensible, assuming `exit` and `abort` are considered "returning". I'd argue they unwind the stack like an exception (that is potentially not captured) so we should be good. 
Sounds good to me, I'll update the code to not drop it; I don't think there's a verifier for this. The only thing is that at first glance it might be surprising to have a function both `willreturn` and `noreturn`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94502