uqbar-project / wollok-ts

TypeScript based Wollok language implementation
GNU General Public License v3.0
20 stars 15 forks source link

Un closure con mensajes que no existen da StackOverflow #269

Closed fdodino closed 1 month ago

fdodino commented 2 months ago

Forma de reproducirlo

wollok:example> [1, 2, 3].min({ a => a.coso() })
✗ Evaluation Error!
  wollok.lang.StackOverflowException wrapping TypeScript RangeError: Maximum call stack size exceeded
wollok:example> [1, 2, 3].filter({ a => a.coso() })
✗ Evaluation Error!
  wollok.lang.StackOverflowException wrapping TypeScript RangeError: Maximum call stack size exceeded

Debería decir a Number does not understand coso()

PalumboN commented 2 months ago

Duplicated https://github.com/uqbar-project/wollok-ts/issues/243

Esto se arregló en este PR https://github.com/uqbar-project/wollok-ts/pull/261

Que al parecer introdujo el issue de perder las const en el REPL https://github.com/uqbar-project/wollok-ts/pull/263#discussion_r1742724942

Hay que revisar todos los casos y tener mejores tests (yo tengo pensado branchear de tu PR que tiene varios tests nuevos y revisar todos los issues del linker)

fdodino commented 2 months ago

Bueno @PalumboN , reapunté wollok-ts-cli y wollok-ts contra el branch fix-#149-repl y anda joya

image

Lo mismo con filter y min:

wollok> [1, 2, 3].filter({ a => a.coso() })
✗ Evaluation Error!
  wollok.lang.MessageNotUnderstoodException: 1 does not understand coso()
wollok> [1, 2, 3].min({ a => a.coso() })
✗ Evaluation Error!
  wollok.lang.MessageNotUnderstoodException: 1 does not understand coso()
wollok> 

Lo mejor es que vos también lo pruebes por las dudas.

fdodino commented 2 months ago

Hmm... @PalumboN revisé bien y me olvidé de reapuntar wollok-ts a la branch fix-repl-149, confirmo que sí está dando Stack Overflow

dodain@local  $  prueba2  ../../wollok-ts-cli/dist/wollok-ts-cli-linux-x64 repl
🖥️  Initializing Wollok REPL on /home/dodain/workspace/wollok-dev/wollok-workspace/prueba2
[WARNING]: shouldDefineConstInsteadOfVar at example.wlk:2
wollok> ✓ Dynamic diagram available at: http://localhost:3000
wollok> [1,2,3].map({ n => n.pepe() })
✗ Evaluation Error!
  wollok.lang.StackOverflowException wrapping TypeScript RangeError: Maximum call stack size exceeded
fdodino commented 2 months ago

Si lo pruebo desde un test funciona ok:

test "al iniciar tiene energia" {
  assert.equals([1, 2].map({ n => n.coso()}), [8, 10])
}
dodain@local  $  prueba2  ../../wollok-ts-cli/dist/wollok-ts-cli-linux-x64 test
🧪 Running all tests on /home/dodain/workspace/wollok-dev/wollok-workspace/prueba2
🌏 Building environment for /home/dodain/workspace/wollok-dev/wollok-workspace/prueba2...

[WARNING]: shouldDefineConstInsteadOfVar at example.wlk:2
Running 1 tests...
 testPepita
   "pepita"
     ✗ "al iniciar tiene energia"

✗ testPepita."pepita"."al iniciar tiene energia"
  wollok.lang.MessageNotUnderstoodException: 1 does not understand coso()
    at testPepita."pepita"."al iniciar tiene energia" [testPepita.wtest:4]

 ✓ 0 passing ✗ 1 failing 
fdodino commented 2 months ago

Te dejo otra cosa que investigué

export function linkSentenceInNode<S extends Sentence>(newSentence: S, context: Node): void {
  const { scope, environment } = context
  // Register top contributions into context's scope
  // scope.register(...scopeContribution(newSentence))
  assignScopes(environment)  // <== este cambio

  // Create scopes for sub-nodes and link nodes (chain)
  newSentence.reduce((parentScope: Scope, node: Node, parent?: Node) => {
    const localScope = new LocalScope(parentScope, ...scopeContribution(node))
    Object.assign(node, { id: uuid(), scope: localScope, environment, parent: parent ?? context })
    return localScope
  }, scope)
}

Fijate que esa línea donde asigna los scopes antes del reduce permite que recuerde las referencias anteriormente creadas y también las variables que están dentro de un closure. El tema es que rompe un test, el que valida que no puedas reasignar una constante. Yo no estoy 100% seguro pero para mí tendríamos que validar dentro de

register(...contributions: [Name, Node][]): void { 

que no te lleguen repetidos y tirar error. Ahora me tengo que ir pero después lo sigo probando.

fdodino commented 1 month ago

@PalumboN @ivojawer , ayer lo probé (y corregí 8 grupos de TP), cierro el issue porque ya está solucionado