[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