Skip to content

Conversation

@DonIsaac
Copy link
Contributor

@DonIsaac DonIsaac commented Apr 8, 2025

What does this PR do?

  • Fix -Werror violations found by clang v20
  • Add source location to bun.assertf

How did you verify your code works?

If it compiles, it works

@robobun
Copy link
Collaborator

robobun commented Apr 8, 2025

Updated 12:43 PM PT - Apr 9th, 2025

@DonIsaac, your commit eaeb798 has 2 failures in Build #14726:


🧪   try this PR locally:

bunx bun-pr 18877

@DonIsaac DonIsaac marked this pull request as draft April 8, 2025 22:00
Comment on lines -4994 to +4995
auto view = WTF::StringView(function->name());
String name = function->name();
auto view = WTF::StringView(name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, what was the error message here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

StringView using a String that was created but immediately thrown away

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, but it's also dubious that we pass it to toZigString right after since that uses the same pointer. What I'm wondering is if the function owns name() (this seems likely, but I haven't investigated fully) or if it's making a new string when we call it.

If it's owned by the function, then this is fine, but if it's a new string then even the changed code is still broken.

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.

5 participants