[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

mattd marked an inline comment 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(),
----------------
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.


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

https://reviews.llvm.org/D55039