Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deduplicate pragma(startAddress) semantic #14512

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

dkorpel
Copy link
Contributor

@dkorpel dkorpel commented Oct 3, 2022

The duplication lead to Issue 11980 being fixed in one place (pragma declarations) but not the other (pragma statements).

@dkorpel dkorpel added the Severity:Refactoring No semantic changes to code label Oct 3, 2022
Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

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

Also why do we even support a pragma statement for start address?
it seems just as useless as a pragma statement for inlining.

Comment on lines +1 to 5
void start()
{
pragma(startaddress, start);
}
pragma(startaddress, start);
Copy link
Contributor

Choose a reason for hiding this comment

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

surely having multiple start addresses should give an error? Or is this fine because its the same function for both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently it doesn't check for it

Copy link
Member

Choose a reason for hiding this comment

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

surely having multiple start addresses should give an error? Or is this fine because its the same function for both?

Who cares, the pragma isn't implemented anyway. ;-)

@dkorpel
Copy link
Contributor Author

dkorpel commented Oct 3, 2022

Also why do we even support a pragma statement for start address?

I asked this on Slack in #dlang. Apparently it's an OMF-only thing. I think we can deprecate it, but I'll ask Walter in the meeting next Friday.

@dkorpel
Copy link
Contributor Author

dkorpel commented Oct 3, 2022

Actually, it does seem to have users (despite most of them being from tests in gcc/gdc forks)

https://github.com/search?l=D&q=pragma%28startaddress&type=Code

We can also keep it supported for legacy, it's not much code

@thewilsonator
Copy link
Contributor

https://github.com/search?l=D&q=pragma%28startaddress&type=Code

Sorry for the confusion, I was talking about the one which you just added, that one is a statement (I think) the existing one is a declaration.

@ibuclaw
Copy link
Member

ibuclaw commented Oct 3, 2022

Actually, it does seem to have users (despite most of them being from tests in gcc/gdc forks)

https://github.com/search?l=D&q=pragma%28startaddress&type=Code

We can also keep it supported for legacy, it's not much code

Ignoring the hundreds of forks of gcc, unique matches I see are:

So I count only one actual user. :-)

@RazvanN7 RazvanN7 merged commit 4c14051 into dlang:master Oct 6, 2022
@dkorpel dkorpel deleted the pragma-startaddress-refactor branch October 6, 2022 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Severity:Refactoring No semantic changes to code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants