aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-exception-at-new.cpp:143 + f_est_noexcept_dependent_used<true>(); +} ---------------- balazske wrote: > balazske wrote: > > aaron.ballman wrote: > > > You have tests for placement new with `nothrow_t`, but I think other > > > forms of placement new are also very interesting to test as those > > > typically don't throw. > > > > > > Additionally, perhaps tests where the allocation functions have been > > > replaced by the user (such as a class allocation function)? > > I realized that the case of user-defined constructor or allocation function > > allows to throw any kind of exception. So the check should be improved to > > handle this case: Not rely on the syntax of new expression, instead check > > if the called allocation function or the called constructor may throw, and > > if yes, check what exceptions are possible. What is your opinion, is it a > > better approach? > > (And a function to collect all possible exceptions called from a function > > is needed, `ExceptionAnalyzer` seems usable.) > It looks like that the user is free to define custom `operator new` and any > constructor called that may throw any exception. Even in the "nothrow" case > it is possible to use a constructor that may throw? If we would analyze every > possible throwable exception that may come out from a new-expression, the > checker would end up almost in a general checker that checks for uncaught > exceptions. At least it is easy to extend. > Not rely on the syntax of new expression, instead check if the called > allocation function or the called constructor may throw, and if yes, check > what exceptions are possible. What is your opinion, is it a better approach? I think that's a better approach. ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-exception-at-new.cpp:143 + f_est_noexcept_dependent_used<true>(); +} ---------------- aaron.ballman wrote: > balazske wrote: > > balazske wrote: > > > aaron.ballman wrote: > > > > You have tests for placement new with `nothrow_t`, but I think other > > > > forms of placement new are also very interesting to test as those > > > > typically don't throw. > > > > > > > > Additionally, perhaps tests where the allocation functions have been > > > > replaced by the user (such as a class allocation function)? > > > I realized that the case of user-defined constructor or allocation > > > function allows to throw any kind of exception. So the check should be > > > improved to handle this case: Not rely on the syntax of new expression, > > > instead check if the called allocation function or the called constructor > > > may throw, and if yes, check what exceptions are possible. What is your > > > opinion, is it a better approach? > > > (And a function to collect all possible exceptions called from a function > > > is needed, `ExceptionAnalyzer` seems usable.) > > It looks like that the user is free to define custom `operator new` and any > > constructor called that may throw any exception. Even in the "nothrow" case > > it is possible to use a constructor that may throw? If we would analyze > > every possible throwable exception that may come out from a new-expression, > > the checker would end up almost in a general checker that checks for > > uncaught exceptions. At least it is easy to extend. > > Not rely on the syntax of new expression, instead check if the called > > allocation function or the called constructor may throw, and if yes, check > > what exceptions are possible. What is your opinion, is it a better approach? > > I think that's a better approach. > Even in the "nothrow" case it is possible to use a constructor that may throw? Certainly -- the `nothrow` syntax is specifying that the allocation cannot throw (it either returns a valid pointer to the allocated memory or null), not that the constructor cannot throw if the allocation succeeds. Is the check intended to care about *allocations* that fail or just exceptions in general around `new` expressions? If it's allocations that fail, then I wouldn't worry about the ctor's exception specification. If exceptions in general, then I agree that we're talking about a general checker for all uncaught exceptions in some ways. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97196/new/ https://reviews.llvm.org/D97196 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
