[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D19201#760358, @Prazek wrote:

> In https://reviews.llvm.org/D19201#760151, @aaron.ballman wrote:
>
> > As an FYI, there is a related check being implemented in clang currently; we probably should not duplicate this effort. See https://reviews.llvm.org/D33333.
>
>
> I think that clang is the right place for this feature, but I am not sure if the patch has all the features (like checking if something will be catched, or not showing warning for conditional noexcepts, which as we have seen
>  could be a problem for some projects. There also might be some other corner cases that we didn't think about. 
>  Assuming that this patch is ready to land, I would propose to send it to trunk and remove it when the clang's patch will make it to the trunk. I am not sure how much time it will take for other patch to be ready, but maybe we could gather some usefull bug reports in the meantime and also Stanislaw would have a contribution.


I don't think it's a good idea to commit this and remove it when the frontend patch lands -- that's just extra code churn and runs the risk of breaking people who are using features out of trunk. I think the better approach is to find the correct home for the functionality based on what *both* patches are trying to accomplish, and combine the test cases from both patches to ensure we get the desired functionality. The two patches appear to be almost entirely overlapping, from my cursory look (of course, I may have missed something).

I think that the frontend is likely the better place for this functionality to go in terms of where users might expect to find it, assuming the false-positive rate is reasonable and it isn't too computationally expensive. My recommendation would be to find the test cases in this patch that are not currently covered by the frontend patch, and ask that they be covered there (possibly even including the fixits).


https://reviews.llvm.org/D19201