Skip to content

Conversation

kkysen
Copy link
Contributor

@kkysen kkysen commented Oct 2, 2025

The workaround for non-portable types for const macros is to insert a use-site cast to the portable type, but this works differently for fn ptrs, so while #1321 is fixed, just skip fn ptr const macros.

This should get #1384 to pass CI by working around this issue there, and it should also workaround #1366, although #1366 in libxml2 appears to have been fixed/worked around from some other change, as #1385 now passes CI.

@kkysen kkysen requested a review from fw-immunant October 2, 2025 09:07
…ortable types

The workaround for non-portable types for const macros
is to insert a use-site cast to the portable type,
but this works differently for fn ptrs,
so while #1321 is fixed, just skip fn ptr const macros.

This should get #1384 to pass CI by working around this issue there,
and it should also workaround #1366,
although #1366 in `libxml2` appears to have been fixed/worked around
from some other change, as #1385 now passes CI.
@kkysen kkysen force-pushed the kkysen/const-macros-skip-fn-ptrs branch from 8979d07 to 8b47edc Compare October 2, 2025 20:50
@fw-immunant
Copy link
Contributor

fw-immunant commented Oct 3, 2025

I don't think this commit alters the behavior of the Py_MEMCPY test case it adds relative to master. I think that's because the scope of #1387 was larger than intended, covering all DeclRefs, not just those that reference variables (cf. functions).

Copy link
Contributor Author

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Ah, this PR used to before the DeclRef PR. I'm pretty sure we had tested that and it fixed that issue. Should we still merge this?

@fw-immunant
Copy link
Contributor

Ah, this PR used to before the DeclRef PR. I'm pretty sure we had tested that and it fixed that issue. Should we still merge this?

I think the motivation behind this is still sound, but a test case for this would need to involve function pointer types without mentioning any decls, which is probably tricky while avoiding casts that are forbidden during const execution.

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