steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land.
LGTM In D124621#3481906 <https://reviews.llvm.org/D124621#3481906>, @mantognini wrote: > One thing I'm not sure about and couldn't easily find in the doc is how to > reference in the commit message the bug (https://llvm.org/PR48534) this patch > fixes. Is it good as is? AFAIK we should prefer GitHub issue numbers to the old BugZilla numbers. Could you please update all references of `48534` -> `47878`. In addition, I think `Fixes #47878` should work in the commit message, and automatically close the given GitHub issue. Please wait for another accept. If you don't get one let's say for a week, just commit it as-is. BTW have you measured the observable impact of this patch on large codebases? Do you have any stats? ================ Comment at: clang/test/Analysis/cxx-member-initializer-const-field.cpp:83 + static int falsePositive1(NonAggregateFP c) { + return 10 / c.i; // Currently, no warning. + } ---------------- ================ Comment at: clang/test/Analysis/cxx-member-initializer-const-field.cpp:87 + static int falsePositive2(NonAggregateFP c) { + return 10 / (c.j - 42); // Currently, no warning. + } ---------------- Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124621/new/ https://reviews.llvm.org/D124621 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
