aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment.
In https://reviews.llvm.org/D33333#760419, @jyu2 wrote: > In https://reviews.llvm.org/D33333#760149, @aaron.ballman wrote: > > > As an FYI, there is a related check currently under development in > > clang-tidy; we probably should not duplicate this functionality in both > > places. See https://reviews.llvm.org/D19201 for the other review. > > > To my understanding, clang-tidy is kind of source check tool. It does more > intensive checking than compiler. > My purpose here is to emit minimum warning form compiler, in case, > clang-tidy is not used. Sort of correct. clang-tidy doesn't necessarily do more intensive checking than the compiler. It's more an AST matching source checking tool for checks that are either expensive or have a higher false-positive rate than we'd want to see in the frontend. I think that this particular check should probably be in the frontend, like you're doing. My biggest concern is that we do not duplicate functionality between the two tools. https://reviews.llvm.org/D19201 has has a considerable amount of review, so you should look at the test cases there and ensure you are handling them (including the fixits). Hopefully your patch ends up covering all of the functionality in the clang-tidy patch. > In https://reviews.llvm.org/D33333#760353, @Prazek wrote: > >> Could you add similar tests as the ones that Stanislaw provied in his patch? >> Like the one with checking if throw is catched, or the conditional noexcept >> (by a macro, etc) > > > Good idea! Could add “marco” test for this. But I am not sure compiler want > to add check for throw caught by different catch handler. Because at compile > time, compiler can not statically determine which catch handler will be used > to caught the exception on all time. I would think that is pragma's > responsibility. > > For example: > > If (a) throw A else throw B; > > > > My main concern there is implicit noexcept gets set by compiler, which cause > runtime to termination. https://reviews.llvm.org/D33333 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
