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.

Sign up for GitHub

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

Jump to bottom

CONVERT_PRIMITIVE no longer has second (optional) argument, why? #3019

Closed
MichalKekely opened this issue Jan 5, 2022 · 3 comments · Fixed by #3020
Closed

CONVERT_PRIMITIVE no longer has second (optional) argument, why? #3019

MichalKekely opened this issue Jan 5, 2022 · 3 comments · Fixed by #3020
Assignees
@mihaibudiu
Labels
question This is a topic requesting clarification.

Comments

@MichalKekely
Copy link
Contributor

The commit/PR #2902 removed the secondary optional argument for CONVERT_PRIMITIVE, that was used to specify priority. The change is from #define CONVERT_PRIMITIVE(NAME, ...) to #define CONVERT_PRIMITIVE(NAME)

Is there any reason why this was removed? Different compilers were using this feature. If there is a reason for removing it, the comment for this macro should be updated as well.
@MichalKekely MichalKekely added the question This is a topic requesting clarification. label Jan 5, 2022
@MichalKekely MichalKekely changed the title CONVERT_PRIMITIVE no longer has second (optional) argument CONVERT_PRIMITIVE no longer has second (optional) argument, why? Jan 5, 2022
@MichalKekely MichalKekely linked a pull request Jan 5, 2022 that will close this issue
Restored optional priority for CONVERT_PRIMITIVE #3020
Merged
@mihaibudiu
Copy link
Contributor

It was removed because there are no users of this feature in the open-source repository.

@MichalKekely
Copy link
Contributor Author

It was removed because there are no users of this feature in the open-source repository.

So could we restore it? Or since it is not used in the open-source repository we don't want to keep it there and it should be redefined elsewhere?

@mihaibudiu
Copy link
Contributor

Of course it can, I just asked for a comment explaining why it's there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mihaibudiu mihaibudiu

Labels
question This is a topic requesting clarification.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Restored optional priority for CONVERT_PRIMITIVE p4lang/p4c
2 participants
@mihaibudiu @MichalKekely