mattd marked 2 inline comments as done.
mattd added inline comments.

================
Comment at: lib/Sema/SemaDecl.cpp:10266
       auto *FPT = NewFD->getType()->castAs<FunctionProtoType>();
-      bool AnyNoexcept = HasNoexcept(FPT->getReturnType());
-      for (QualType T : FPT->param_types())
-        AnyNoexcept |= HasNoexcept(T);
-      if (AnyNoexcept)
+      if (hasNoexcept(FPT->getReturnType()) ||
+          llvm::any_of(FPT->param_types(),
----------------
mattd wrote:
> Rakete1111 wrote:
> > What you're doing here is a subset of what `hasNoexcept` is doing for 
> > function types. Do you think it would make sense to use `!FPT->isNothrow() 
> > && hasNoexcept(NewFW->getType())` or something like that?
> Thanks for the comments @Rakete1111.
> 
> I do see the similarity and was trying to implement a solution to take 
> advantage of that.  However, I was running into trouble on assert/debug 
> builds.  `llvm_unreachable`, called in `FunctionProtoType::canThrow`, will 
> trigger on FunctionProtoType instances that have not yet had their 
> exception-spec type evaluated.  `canThrow` is called on behalf of 
> `isNothrow`.  Some of the test cases will fail in assert/debug builds if we 
> place a call to to `FPT->isNothrow` here. 
> 
> The original code avoided this assertion by not calling `isNothrow` on FPT 
> here, but instead just evaluating the return and parameter types of FPT.  I 
> made this patch similar to what was already in place, but with the more  
> thorough check in `hasNoexcept.`  I'd be pleased to remove this 
> duplication/similarity if I can avoid this assertion.
I just got back into looking at this issue.  Given that I was unable to 
implement `!FPT->isNoThrow() && hasNoexcept(NewFW->getType())` without 
triggering an assertion (discussed in my reply immediately above), is there 
anything we should be doing differently here?  The change, as it sits now (with 
the duplication), seems to work fine.


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

https://reviews.llvm.org/D55039



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

Reply via email to