Skip to content

Refactor: Replace custom getOrigin() method with existing getOldInput… - #1585

Open
Rachit Mehta (rachitmeht) wants to merge 1 commit into
qualcomm:mainfrom
rachitmeht:refactor/unify-common-symbol-origin-tracking
Open

Refactor: Replace custom getOrigin() method with existing getOldInput…#1585
Rachit Mehta (rachitmeht) wants to merge 1 commit into
qualcomm:mainfrom
rachitmeht:refactor/unify-common-symbol-origin-tracking

Conversation

@rachitmeht

@rachitmeht Rachit Mehta (rachitmeht) commented Jul 25, 2026

Copy link
Copy Markdown

CommonELFSection had a custom getOrigin() method while other sections
used the virtual getOldInputFile(). This forced type-specific code:

if (auto *CS = dyn_cast<CommonELFSection>(sect))
  input = CS->getOrigin();
if (sect->getOldInputFile())
  input = sect->getOldInputFile();

Now CommonELFSection overrides getOldInputFile(), enabling uniform code:

if (sect->hasOldInputFile())
  input = sect->getOldInputFile();

Changes:

  • Add hasOldInputFile/getOldInputFile overrides to CommonELFSection
  • Remove redundant getOrigin method
  • Simplify ObjectBuilder, SectionMap, GarbageCollection call sites
  • Replace dyn_cast + type-specific calls with polymorphic interface

Resolves #1300

@parth-07 Parth (parth-07) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please improve the commit message here? This resource might be helpful: https://cbea.ms/git-commit/

Comment thread include/eld/Readers/CommonELFSection.h Outdated
@rachitmeht
Rachit Mehta (rachitmeht) force-pushed the refactor/unify-common-symbol-origin-tracking branch 2 times, most recently from 0186baf to 8185108 Compare August 4, 2026 05:01
I = CommonSection->getOrigin()->getInput();
IsCommonSection = true;

if (Section->hasOldInputFile()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were we previously using old input file for non-common sections?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we were not, but as I variable is only used for diagnostic outputs, so it is better to use origin file only irrespective of common sections.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A refactoring patch should not introduce functionality change, even a diagnostic change. If you think this change is helpful, can you open a separate issue to discuss it?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed and opened a seperate issue for the change.

@quic-areg quic-areg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Steven6798

Copy link
Copy Markdown
Contributor

There are multiple commits and some of them are not signed. Please add all changes into a single commit since the fix is small enough.

InputFile *getOrigin() const { return Origin; }
bool hasOldInputFile() const override { return Origin != nullptr; }

InputFile *getOldInputFile() const override { return Origin; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably another TODO is, rename getOldInputFile function to

getPreLtoInputFile or getOriginalInputFile ?

CommonELFSection had a custom getOrigin() method while other sections
used the virtual getOldInputFile(). This forced type-specific code:

    if (auto *CS = dyn_cast<CommonELFSection>(sect))
      input = CS->getOrigin();
    if (sect->getOldInputFile())
      input = sect->getOldInputFile();

Now CommonELFSection overrides getOldInputFile(), enabling uniform code:

    if (sect->hasOldInputFile())
      input = sect->getOldInputFile();

Changes:
- Add hasOldInputFile/getOldInputFile overrides to CommonELFSection
- Remove redundant getOrigin method
- Simplify ObjectBuilder, SectionMap, GarbageCollection call sites
- Replace dyn_cast + type-specific calls with polymorphic interface

Resolves qualcomm#1300

Signed-off-by: Rachit Mehta <rachmeht@qti.qualcomm.com>
@rachitmeht
Rachit Mehta (rachitmeht) force-pushed the refactor/unify-common-symbol-origin-tracking branch from e022705 to 3378cf9 Compare August 12, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CommonELFSection should override getOldInputFile

5 participants