[PATCH] D78251: [MC] Replace MCSection*::getName() with MCSection::getName(). NFC

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 15 18:46:48 PDT 2020


MaskRay added inline comments.


================
Comment at: llvm/include/llvm/MC/MCSection.h:104
+  // TODO Make Name private after we stop supporting generation of GNU-style
+  // .zdebug_* sections.
+  StringRef Name;
----------------
rnk wrote:
> Are we sure we are dropping .zdebug_* support? If we're not confident, maybe just say "TODO: Make this private when possible."
Thanks, I'll reword it.

grimar asked whether we can drop accepting `.zdebug_*` as input.

I mentioned D61689 (which was reverted because we are relying on it... Eventually it is because of some integrated assembler difficulties)




================
Comment at: llvm/include/llvm/MC/MCSectionELF.h:50
 
-  MCSectionELF(StringRef Section, unsigned type, unsigned flags, SectionKind K,
+  // The storage of Name is owned by TargetLoweringObjectFileELF's ELFUniqueMap.
+  MCSectionELF(StringRef Name, unsigned type, unsigned flags, SectionKind K,
----------------
rnk wrote:
> Isn't it owned by `MCContext::ELFUniquingMap`? That's much less surprising, since MCContext is the class responsible for creating these objects.
Thanks! Looks like the original comment was wrong..


================
Comment at: llvm/include/llvm/MC/MCSectionELF.h:63
+  // sections.
+  void setSectionName(StringRef Name) { this->Name = Name; }
 
----------------
rnk wrote:
> O_O exciting
For your amusement, this is a hack only used by the `.debug_* -> .zdebug_*` renaming.


================
Comment at: llvm/include/llvm/MC/MCSectionWasm.h:44
 
+  // The storage of Name is owned by TargetLoweringObjectFileWasm's WasmUniqueMap.
   friend class MCContext;
----------------
rnk wrote:
> I think it's MCContext again, I guess the ownership changed.
It is a WasmSectionKey element in WasmUniquingMap (which is in MCContext)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78251





More information about the llvm-commits mailing list