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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to