Skip to content

Conversation

@vrn-sn
Copy link
Member

@vrn-sn vrn-sn commented Oct 24, 2025

Problem

Multiple runtime APIs incorrectly were using std::string::reserve where they should have been using std::string::resize. This led to UB that seemed to behave fine at runtime, but the introduction of #467 caused all of these APIs to break.

Solution

We introduce uvutils::getStringFromUv as a utility API to help reduce errors when interacting with libuv. Internally, this handles all the buffer logic and simply returns a value or an error to the caller.

In fixing this, I also discovered two issues with our CI:

  1. One of the checks in path.posix.test.luau is flipped but was mistakenly passing because of the previously broken process.cwd() API.
  2. Our Luau tests don't even run on Windows! 😞

I've fixed (1) and have made a separate issue for (2): #483.

@vrn-sn vrn-sn changed the title Fix working directory error in docs deploy job Fix multiple runtime APIs that were incorrectly using std::string::reserve Oct 25, 2025
@vrn-sn vrn-sn requested a review from aatxe October 25, 2025 04:56
Copy link
Collaborator

@Vighnesh-V Vighnesh-V left a comment

Choose a reason for hiding this comment

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

Requesting some small changes, but this broadly looks good to go.

@vrn-sn
Copy link
Member Author

vrn-sn commented Oct 27, 2025

Currently also trying to figure out why Windows tests are failing now that I'm trying to re-enable them.

Edit: giving up on this for now and reverting these Windows-related changes.

@vrn-sn vrn-sn requested a review from Vighnesh-V October 27, 2025 18:29
Copy link
Collaborator

@Vighnesh-V Vighnesh-V left a comment

Choose a reason for hiding this comment

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

Some style comments, but not blocking.

@vrn-sn vrn-sn enabled auto-merge (squash) October 27, 2025 18:52
@vrn-sn vrn-sn merged commit f27ec6f into primary Oct 27, 2025
10 checks passed
@vrn-sn vrn-sn deleted the vrn-sn/try-fixing-wd-in-docs-job branch October 27, 2025 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants