[PATCH] D78929: [AIX][XCOFF]emit extern linkage for the llvm intrinsic symbol

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1756
+  bool Ret = AsmPrinter::doFinalization(M);
+  for (auto Sym : IntrinsicSymbols)
+    OutStreamer->emitSymbolAttribute(Sym, MCSA_Extern);
----------------
DiggerLin wrote:
> jhenderson wrote:
> > Don't use `auto` here. Also, should this really be a copy and not a const reference?
> it store the a pointer of MCSymbol in the IntrinsicSymbols. I think return a copy of pointer is same efficient as const reference. 
> 
> and the 
> 
> ```
> bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym,
>                                           MCSymbolAttr Attribute)
> ```
One issue with `auto` here is that it's not obvious that we have a pointer type (and even knowing how `Sym` is used does not rule out the possibility that it is merely convertible to the pointer type). Using `auto *` might not have caused the comment to be raised, but since the comment has been raised, please get rid of the `auto`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929