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

Marco Antognini via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 29 09:05:59 PDT 2022


mantognini marked 2 inline comments as done.
mantognini added a comment.

In D124621#3481973 <https://reviews.llvm.org/D124621#3481973>, @steakhal wrote:

> 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.

Thanks for the feedback!

> BTW have you measured the observable impact of this patch on large codebases? Do you have any stats?

I can't share the data but I can say it fixes some user reports. :-)

In D124621#3481981 <https://reviews.llvm.org/D124621#3481981>, @steakhal wrote:

> Noq wrote https://github.com/llvm/llvm-project/issues/47878#issuecomment-981036634
>
>> Aha, uhm, yeah, i see. The static analyzer indeed thinks that a combination of "const" and a field initializer causes the field to forever stay that way. **We'll need to undo this relatively recently added shortcut.**
>
> What "recently added shortcut" did he mention? Could you please refer to that commit in the patch summary, please?

I think @NoQ refers to https://reviews.llvm.org/D45774 but I'll wait for a week or so for confirmation in case there's more to it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124621/new/

https://reviews.llvm.org/D124621



More information about the cfe-commits mailing list