This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by MaskRay on Apr 15 2020, 3:57 PM.

Details

Summary

I plan to use MCSection::getName() in D78138. Having the function in the base class is also convenient for debugging.

Diff Detail

Event Timeline

MaskRay created this revision.Apr 15 2020, 3:57 PM
rnk added inline comments.Apr 15 2020, 4:17 PM
llvm/include/llvm/MC/MCSectionMachO.h
25–26

This data structure choice seems like the only thing preventing us from moving SectionName to the MCSection base class, and having a vanilla, non-virtual getName accessor. Want to try that instead? The only complication is that you have to stabilize the StringRef by using a substring of the string key in the MachOUniquingMap.

MaskRay updated this revision to Diff 257911.Apr 15 2020, 5:34 PM
MaskRay retitled this revision from [MC] Add virtual member function MCSection::getName(). NFC to [MC] Replace MCSection*::getName() with MCSection::getName(). NFC.
MaskRay edited the summary of this revision. (Show Details)

Fix MCSectionMachO as suggested by @rnk

MaskRay marked an inline comment as done.Apr 15 2020, 5:35 PM
rnk accepted this revision.Apr 15 2020, 6:04 PM

lgtm, just comments on comments.

llvm/include/llvm/MC/MCSection.h
104

Are we sure we are dropping .zdebug_* support? If we're not confident, maybe just say "TODO: Make this private when possible."

llvm/include/llvm/MC/MCSectionELF.h
54

Isn't it owned by MCContext::ELFUniquingMap? That's much less surprising, since MCContext is the class responsible for creating these objects.

66

O_O exciting

llvm/include/llvm/MC/MCSectionWasm.h
48

I think it's MCContext again, I guess the ownership changed.

This revision is now accepted and ready to land.Apr 15 2020, 6:04 PM
MaskRay updated this revision to Diff 257933.Apr 15 2020, 6:31 PM
MaskRay marked 6 inline comments as done.

Address comments

Thanks for review!

llvm/include/llvm/MC/MCSection.h
104

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)

llvm/include/llvm/MC/MCSectionELF.h
54

Thanks! Looks like the original comment was wrong..

66

For your amusement, this is a hack only used by the .debug_* -> .zdebug_* renaming.

llvm/include/llvm/MC/MCSectionWasm.h
48

It is a WasmSectionKey element in WasmUniquingMap (which is in MCContext)

This revision was automatically updated to reflect the committed changes.