This is an archive of the discontinued LLVM Phabricator instance.

[SystemZ][z/OS] Remove ENDRecord class from GOFF.h
Needs ReviewPublic

Authored by yusra.syeda on May 4 2023, 1:07 PM.

Details

Summary

ENDRecord class is no longer needed.

Diff Detail

Event Timeline

yusra.syeda created this revision.May 4 2023, 1:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 1:07 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
yusra.syeda requested review of this revision.May 4 2023, 1:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 1:07 PM
yusra.syeda retitled this revision from [SystemZ][z/OS] Remove ENDRecord class from GOFFObjectFile to [SystemZ][z/OS] Remove ENDRecord class from GOFF.h.May 4 2023, 1:07 PM
Stoorx added a subscriber: Stoorx.May 5 2023, 12:09 AM

Why? Is it a piece of dead-code or it is never used (and surely will never be)?

Please provide some reasoning.

Why? Is it a piece of dead-code or it is never used (and surely will never be)?

Please provide some reasoning.

Yes, this is dead code.

kpn added a comment.May 17 2023, 10:35 AM

Why delete from class ENDRecord the support for having the entry point specified on the END card? Wouldn't that be useful in a dumping tool at least? Or is this handled somewhere else now?

Anything the ENDRecord does can be easily handled by the parent Record class.

The ENDRecord class only has two functions, getNameLength is just a wrapper for get<T>() in the parent class (defined here) and getData() is likewise a basically just a wrapper for getContinuousData(), also defined in the parent class.

kpn added a comment.Aug 1 2023, 10:47 AM

Anything the ENDRecord does can be easily handled by the parent Record class.

The ENDRecord class only has two functions, getNameLength is just a wrapper for get<T>() in the parent class (defined here) and getData() is likewise a basically just a wrapper for getContinuousData(), also defined in the parent class.

There are seven different fields in the END record. Having END cards be the only record type that requires callers to unpack the record manually doesn't seem like good design.

And the class is only dead because it hasn't been connected yet.