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

refactor use primordials in extensions/webgpu #11265

Merged
merged 2 commits into from
Jul 4, 2021

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju mentioned this pull request Jul 4, 2021
67 tasks
Copy link
Member

@satyarohith satyarohith left a comment

Choose a reason for hiding this comment

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

WebGPU flies are huge. I should've checked the line count of the files before I started the review. 😄

Nice work, Bartek. I couldn't find a single instance unaddressed.

LGTM.

@SimonRask
Copy link
Contributor

SimonRask commented Jul 4, 2021

Found some things in 01_webgpu.js:

  • line 615
  • line 1761
  • line 1313

Found nothing in 02_idl_types.js

@satyarohith
Copy link
Member

Thanks for taking a look, @SimonRask!

@bartlomieju bartlomieju merged commit ae526e0 into denoland:main Jul 4, 2021
@bartlomieju bartlomieju deleted the primordials_webgpu branch July 4, 2021 14:23
@ghost
Copy link

ghost commented Jul 4, 2021

There were a few accesses on TypedArray/DataView objects that aren't using primordials here, e.g. directly accessing the .buffer property.

@bartlomieju
Copy link
Member Author

There were a few accesses on TypedArray/DataView objects that aren't using primordials here, e.g. directly accessing the .buffer property.

@crimsoncodes0 I believe this is the same situation as with .length. Node doesn't really use primordials for such accesses.

@ghost
Copy link

ghost commented Jul 4, 2021

There were a few accesses on TypedArray/DataView objects that aren't using primordials here, e.g. directly accessing the .buffer property.

@crimsoncodes0 I believe this is the same situation as with .length. Node doesn't really use primordials for such accesses.

String length are array length properties are safe to access, as that is the only way to access them. String have an immutable own property length, while arrays have a "magic" own property that has setting intercepted by the engine to cast the written value to an unsigned integer.

TupedArray properties are very different, in that they aren't nearly as safe, nor are they own properties. %TypedArray%.prototype.{buffer, byteLength, length} are all getters that exist on the %TypedArray% prototype, thus are able to be mutated by user code.

If it would be appreciated, I can make some PRs tommorow to use them across the JavaScript.

@bartlomieju
Copy link
Member Author

@crimsoncodes0 thanks explanation. Please do open a PR.

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.

None yet

3 participants