Skip to content

Prevent use of primitive float operation functions#763

Merged
asherkin merged 7 commits intoalliedmodders:masterfrom
Headline:people-still-use-these
Feb 2, 2018
Merged

Prevent use of primitive float operation functions#763
asherkin merged 7 commits intoalliedmodders:masterfrom
Headline:people-still-use-these

Conversation

@Headline
Copy link
Member

@Headline Headline commented Feb 2, 2018

Apparently people use the float natives for actual float arithmetic; they're not really intended to be used directly. I've added a note to deter people from using these instead of their respective operators.

@asherkin
Copy link
Member

asherkin commented Feb 2, 2018

I'd prefer "This native is internal implementation." rather than "This native only exists to be overloaded."

Does adding #deprecated work and not carry through to the operators?

@Headline
Copy link
Member Author

Headline commented Feb 2, 2018

@asherkin I haven’t tested whether #depreicated will carry through, but I was under the impression that depreicated implies removal so I’m not sure if that’s what we’d want to suggest.

I suppose deprecation is only correlated with removal, so I redact that.

@asherkin
Copy link
Member

asherkin commented Feb 2, 2018

We want people to stop using these natives. They are not used internally in modern plugins, the JIT emits float opcodes directly, they're definitely on the removal path if we break binary compat for some reason.

@Headline
Copy link
Member Author

Headline commented Feb 2, 2018

The deprecation warning doesn't carry through to the overloads, so #pragma deprecated works as we'd want here. I also threw in your edit, thanks!

🚢

Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

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

I look forward to the :steamsalty:

@asherkin asherkin merged commit 12fca79 into alliedmodders:master Feb 2, 2018
@Headline Headline deleted the people-still-use-these branch February 3, 2018 03:16
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.

2 participants