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

another try at adding AddAnyAttr to AnyView #3458

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

gbj
Copy link
Collaborator

@gbj gbj commented Jan 8, 2025

@luxalpa had the very reasonable suggestion that perhaps the #[inline(always)] annotation that I've removed here was the problem with the AnyView::add_any_attr() implementation all along. This makes sense to me intuitively as I could see how it would lead to a mutually-recursive inlined function that would be... very big.

I don't have a good test case and can no longer compile the test case that you shared with me @zakstucke, so I'm wondering if some others could try it out to see if this fixes the infinite compile time issues with that change.

See #3156 and discussion beginning here for context.

@zakstucke
Copy link
Contributor

zakstucke commented Jan 8, 2025

Not looking good :(

After 10 mins of compiling:

cannot encode offset of relocations; object file too large

This error is raised from this compiler code:
https://llvm.org/doxygen/MachObjectWriter_8cpp_source.html

i've also tested by removing all #[inline(always)] across leptos - no bueno either (although it does seem to improve compile times substantially when not adding the AnyView anyattr stuff)

@geoffreygarrett
Copy link

Would it be agreeable to feature gate this under any-view-any-attr for now, while we work on getting it fully functional? I haven't encountered any issues on my end.

@zakstucke
Copy link
Contributor

See referenced PR, different implementation but same problem (I was hoping traits and hoisting the downcast out might fix it). It might narrow down the issue.

@geoffreygarrett
Copy link

@zakstucke, would you mind giving #3461 a spin on your failing case to see if it helps?

@zakstucke
Copy link
Contributor

zakstucke commented Jan 8, 2025

No change unfortunately - attempt and testing I did with #3460 seems to show the issue being add_any_attr(attr) rather than nested into_any()

@geoffreygarrett
Copy link

Leaving this here for traceability 7fc0c41. Let me what effect it has, if any.

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.

3 participants