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

add git_jit_type_is_float() #51

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

zedar
Copy link

@zedar zedar commented Apr 22, 2024

No description provided.

Copy link

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Please add documentation in gcc/jit/docs/topics/types.rst and tests.

The rest is good, thanks!

@zedar
Copy link
Author

zedar commented Apr 23, 2024

Please add documentation in gcc/jit/docs/topics/types.rst and tests.

The rest is good, thanks!

Added more unit tests and updated description

Copy link

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

One last thing. Sorry, I didn't think about this earlier.

.. function:: int\
gcc_jit_type_is_float (gcc_jit_type *type)

Return non-zero if the type is a float.
Copy link

Choose a reason for hiding this comment

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

Now that I see this, I believe we should rename the function to make sure people do not think that this checks that this is the float type in C.
I would rename every float to floating_point (for instance gcc_jit_type_is_floating_point) unless you have a better idea.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, let's change it to gcc_jit_type_is_floating_point()

@@ -60,6 +105,7 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
CHECK_VALUE (gcc_jit_vector_type_get_element_type(vector_type), double_type);
CHECK_VALUE (gcc_jit_vector_type_get_num_units(vector_type), 4);
CHECK (!gcc_jit_type_is_integral(vec_type));
CHECK (gcc_jit_type_is_float(vec_type));
Copy link

Choose a reason for hiding this comment

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

I'm not sure this makes sense that this returns true on a vector type.
Is this the same for gcc_jit_type_is_integral?

Copy link
Author

Choose a reason for hiding this comment

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

vector_type inherits from decorated_type, whose is_float method calls the is_float() method of the vector element.
I will add additional check for vectors.

@zedar zedar force-pushed the add_git_jit_type_is_float branch 2 times, most recently from 09a6029 to d3ac970 Compare April 23, 2024 20:03
Copy link

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

One last nitpick.

@@ -604,7 +604,7 @@ int
gcc_jit_type_is_bool (gcc_jit_type *type)
{
RETURN_VAL_IF_FAIL (type, FALSE, NULL, NULL, "NULL type");

Copy link

Choose a reason for hiding this comment

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

Please remove this change.

Copy link
Author

Choose a reason for hiding this comment

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

Removed

For gcc_jit_type_is_float() add unit tests and function description

Rename gcc_jit_type_is_float to gcc_jit_type_is_floating

improve description

Remove unecessary spaces
@zedar zedar force-pushed the add_git_jit_type_is_float branch from d3ac970 to a6d92cb Compare April 24, 2024 06:16
@antoyo antoyo merged commit a476be0 into rust-lang:master Apr 24, 2024
2 checks passed
@antoyo
Copy link

antoyo commented Apr 24, 2024

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants