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

audit AOs with optional parameters #3475

Open
5 of 41 tasks
michaelficarra opened this issue Nov 15, 2024 · 6 comments
Open
5 of 41 tasks

audit AOs with optional parameters #3475

michaelficarra opened this issue Nov 15, 2024 · 6 comments
Labels
editor call to be discussed in the next editor call

Comments

@michaelficarra
Copy link
Member

michaelficarra commented Nov 15, 2024

For example, the Call AO's argument list parameter is currently optional, which can make it a footgun for authors and a bit misleading for readers. We should make it mandatory. See tc39/proposal-upsert#65.

Matrix conversation

reviewed:

  • ToPrimitive preferredType
  • Call argumentsList
  • Construct argumentsList
  • Construct newTarget
  • CreateListFromArrayLike validElementTypes
  • Invoke argumentsList
  • IteratorNext value
  • ResolveBinding env
  • OrdinaryObjectCreate additionalInternalSlotsList
  • OrdinaryCreateFromConstructor internalSlotsList
  • MakeConstructor writablePrototype
  • MakeConstructor prototype
  • SetFunctionName prefix
  • CreateBuiltinFunction realm
  • CreateBuiltinFunction prototype
  • CreateBuiltinFunction prefix
  • ArrayCreate proto
  • ForIn/OfBodyEvaluation iteratorKind

not yet reviewed:

  • InstantiateOrdinaryFunctionExpression name
  • InstantiateArrowFunctionExpression name
  • DefineMethod functionPrototype
  • InstantiateGeneratorFunctionExpression name
  • InstantiateAsyncGeneratorFunctionExpression name
  • InstantiateAsyncFunctionExpression name
  • InstantiateAsyncArrowFunctionExpression name
  • LoadRequestedModules hostDefined
  • GetExportedNames exportStarSet
  • ResolveExport resolveSet
  • ExecuteModule capability
  • FlattenIntoArray mapperFunction
  • FlattenIntoArray thisArg
  • AllocateTypedArray length
  • AllocateArrayBuffer maxByteLength
  • DetachArrayBuffer key
  • GetValueFromBuffer isLittleEndian
  • SetValueInBuffer isLittleEndian
  • AllocateSharedArrayBuffer maxByteLength
  • ValidateAtomicAccessOnIntegerTypedArray waitable
  • PerformPromiseThen resultCapability
  • CreateIteratorFromClosure extraSlots
  • AsyncGeneratorCompleteStep realm
@ljharb
Copy link
Member

ljharb commented Nov 15, 2024

I assume the goal here is to remove footguns, not to remove optional parameters entirely?

@michaelficarra
Copy link
Member Author

It's to consider whether each optional parameter is a net negative or a net positive and remove the former.

@michaelficarra
Copy link
Member Author

An example of OrdinaryObjectCreate's optional parameter being forgotten: tc39/proposal-esm-phase-imports#39

@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Nov 26, 2024
@michaelficarra
Copy link
Member Author

michaelficarra commented Feb 10, 2025

For OrdinaryObjectCreate, it may be a bit too burdensome to make the optional parameter mandatory, but it would be valuable to require it so that it isn't forgotten. We're going to leave it out of this list for now, but explore lifting the internalSlotsList to all the call sites to make it even more explicit, as a separate PR. OrdinaryCreateFromConstructor should match.

@bakkot
Copy link
Contributor

bakkot commented Feb 10, 2025

For MakeConstructor, we should replace the existing optional params with a two-state "is a class" vs "is not a function" (or possibly "writable prototype" vs "nonwriteable prototype". This could be a boolean but an enum would be better probably. Then we can do the nonwriteable-prototype stuff inside of ClassDefinitionEvaluation.

@michaelficarra
Copy link
Member Author

CreateBuiltinFunction could use a more extensive refactoring that may include splitting it or changing the parameter list. Out of scope for this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor call to be discussed in the next editor call
Projects
None yet
Development

No branches or pull requests

3 participants