[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