Rakete1111 added a comment.
Apart from the comments I think your patch LGTM, but I'll let someone else hav
the final say.
================
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:
> 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.
I think it's fine.
================
Comment at: lib/Sema/SemaDecl.cpp:10268
+ llvm::any_of(FPT->param_types(),
+ [](QualType Q) { return hasNoexcept(Q); })) {
Diag(NewFD->getLocation(),
----------------
Rakete1111 wrote:
> Same, that lambda is unnecessary.
You marked this comment as done but the lambda is still there :).
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
