isuckatcs added a comment.
> "The warning was emitted at every occurence of the function. It might be
> confusing if it's only emitted for the definition."
> Why ? Issue is in definition, not declaration.
For me it would be confusing, because the forward declaration is naming the
same function as the definition.
If I see that something is reported for the definition but not for the
declaration I might think it's a bug in the
tool, like once there is a problem with the function and the other time there
isn't. Note that this particular
warning is reported for the function and not for something inside the
definition.
Also we have cross translation unit analysis, though I'm not sure this
particular checker works there too.
Assuming it does, what happens if I forward declare the function in one
translation unit and define it in
a different one? With this change the warning will only be output in the
translation unit,the function is
defined in and this might silently hide some other problems in the TU the
function is forward declared in.
> recursion_helper does not throw, it crashes.
Technically the exception is propagated through the function until a handler is
found that catches it.
> Example with indirectly_recursive & recursion_helper behave in same way, only
> difference is that warning is emitted only for definition.
Please add a test case for this regardless of the behaviour to see that the
checker handles exception propagation.
> This is other bug that I'm fixing (not under this patch) related to properly
> handling noexcept keyword.
I'm not sure what you mean by bug here.
int recursion_helper(int n) noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in
// function 'recursion_helper' which should not throw exceptions
indirectly_recursive(n);
}
int indirectly_recursive(int n) noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in
// function 'indirectly_recursive' which should not throw exceptions
if (n == 0)
throw n;
return recursion_helper(n);
}
Because `recursion_helper()` propagates the thrown object it makes sense to
emit a warning for that too.
Also because the warning is emitted upon every propagation it is easy to trace
where the exception actually
comes from. Think of it like a stack trace for example.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148462/new/
https://reviews.llvm.org/D148462
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
