jyu2 added a comment. In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:
> In https://reviews.llvm.org/D33537#771159, @baloghadamsoftware wrote: > > > In https://reviews.llvm.org/D33537#770264, @aaron.ballman wrote: > > > > > I think we should try to get as much of this functionality in > > > https://reviews.llvm.org/D33333 as possible; there is a considerable > > > amount of overlap between that functionality and this functionality. This > > > check can then cover only the parts that are not reasonably handled by > > > the frontend check instead of duplicating diagnostics the user already > > > receives. > > > > > > I suppose that the frontend check will not travarse the call-graph just > > check direct throws. Should we only check indirect throws then? > > > The check in https://reviews.llvm.org/D33333 is using a CFG, not just > checking direct throws. In https://reviews.llvm.org/D33537#807997, @erichkeane wrote: > In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote: > > > In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote: > > > > > The check in https://reviews.llvm.org/D33333 is using a CFG, not just > > > checking direct throws. > > > > > > I tested the latest revision (the fronted patch already included) on my > > test file. Disregarding of the not so important parameters > > (`EnabledFunctions` and `IgnoredExceptions`) I do not get warnings for any > > indirect throws (`indirect_implicit()` and `indirect_explicit()`). So for > > me it does not seem to be using the CFG. Furthermore, I do not get warning > > for `throw_and_catch_some()` where `1.1` is a `double` thus `catch(int &)` > > should not catch it. The same happens in `throw_catch_rethrow_the_rest()`, > > where `catch(int &)` should not catch `1.1`, but `catch(...)` should catch > > and rethrow it. This latter may be a bug. > > > Jen Yu can clarify, but I believe it was decided to put the implementation in > the CFG, but not do a full analysis (leave that for a later implementation). > It is not doing 'catch' analysis, and I don't believe they decided to do > analysis on the function call itself, since the false-positive rate is > massive thanks to the C headers. The catch analys In https://reviews.llvm.org/D33537#807997, @erichkeane wrote: > In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote: > > > In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote: > > > > > The check in https://reviews.llvm.org/D33333 is using a CFG, not just > > > checking direct throws. > > > > > > I tested the latest revision (the fronted patch already included) on my > > test file. Disregarding of the not so important parameters > > (`EnabledFunctions` and `IgnoredExceptions`) I do not get warnings for any > > indirect throws (`indirect_implicit()` and `indirect_explicit()`). So for > > me it does not seem to be using the CFG. Furthermore, I do not get warning > > for `throw_and_catch_some()` where `1.1` is a `double` thus `catch(int &)` > > should not catch it. The same happens in `throw_catch_rethrow_the_rest()`, > > where `catch(int &)` should not catch `1.1`, but `catch(...)` should catch > > and rethrow it. This latter may be a bug. > > > Jen Yu can clarify, but I believe it was decided to put the implementation in > the CFG, but not do a full analysis (leave that for a later implementation). > It is not doing 'catch' analysis, and I don't believe they decided to do > analysis on the function call itself, since the false-positive rate is > massive thanks to the C headers. In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote: > > > The check in https://reviews.llvm.org/D33333 is using a CFG, not just > > checking direct throws. > > > I tested the latest revision (the fronted patch already included) on my test > file. Disregarding of the not so important parameters (`EnabledFunctions` and > `IgnoredExceptions`) I do not get warnings for any indirect throws > (`indirect_implicit()` and `indirect_explicit()`). So for me it does not seem > to be using the CFG. Furthermore, I do not get warning for > `throw_and_catch_some()` where `1.1` is a `double` thus `catch(int &)` should > not catch it. The same happens in `throw_catch_rethrow_the_rest()`, where > `catch(int &)` should not catch `1.1`, but `catch(...)` should catch and > rethrow it. This latter may be a bug. In https://reviews.llvm.org/D33537#807997, @erichkeane wrote: > In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote: > > > In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote: > > > > > The check in https://reviews.llvm.org/D33333 is using a CFG, not just > > > checking direct throws. > > > > > > I tested the latest revision (the fronted patch already included) on my > > test file. Disregarding of the not so important parameters > > (`EnabledFunctions` and `IgnoredExceptions`) I do not get warnings for any > > indirect throws (`indirect_implicit()` and `indirect_explicit()`). So for > > me it does not seem to be using the CFG. Furthermore, I do not get warning > > for `throw_and_catch_some()` where `1.1` is a `double` thus `catch(int &)` > > should not catch it. The same happens in `throw_catch_rethrow_the_rest()`, > > where `catch(int &)` should not catch `1.1`, but `catch(...)` should catch > > and rethrow it. This latter may be a bug. > > > Jen Yu can clarify, but I believe it was decided to put the implementation in > the CFG, but not do a full analysis (leave that for a later implementation). > It is not doing 'catch' analysis, and I don't believe they decided to do > analysis on the function call itself, since the false-positive rate is > massive thanks to the C headers. >>>> The patch do not do indirect function call check, since that would cause >>>> false-positive. >>>> The tests of throw_and_catch_some and throw_catch_rethrow_the_rest no >>>> warning emit due the "throw 1.1" is unreachable code after "throw 1", it >>>> is not the bug. I am not sure clang has some option to emit unreachable >>>> code. https://reviews.llvm.org/D33537 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
