[PATCH] D124621: CPP-2461 [Analyzer] Fix assumptions about const field with member-initializer

[Balázs Benics via Phabricator via cfe-commits]

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