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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits