[PATCH] D133158: [NFC] Make MultiplexExternalSemaSource own sources
Adrian Prantl via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 1 16:49:29 PDT 2022
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
This looks plausible to me — did you build clang-tools-extra & lldb to make sure nothing else needs to be updated?
================
Comment at: clang/include/clang/Sema/MultiplexExternalSemaSource.h:53
///
- MultiplexExternalSemaSource(ExternalSemaSource& s1, ExternalSemaSource& s2);
+ MultiplexExternalSemaSource(ExternalSemaSource* S1, ExternalSemaSource* S2);
----------------
beanz wrote:
> aprantl wrote:
> > why this change? Does `&` imply ownership in our coding style?
> The old implementation took the address of the references to store. It seems more clear to me to accept a pointer when you need a pointer.
>
> The old interface using references also allowed for passing in stack-allocated objects, which causes problems since the change here calls retain and release on the underlying ref count object. Taking pointers puts the onus on the user of the API to think through where the address passed in comes from.
nit: clang-format :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133158/new/
https://reviews.llvm.org/D133158
More information about the cfe-commits
mailing list