doesNotUseRecursion fix #947
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Muy bien análisis! Gracias por la mano! Con respecto a los count, qué quieren hacer? Podríamos hacer nuestro propio count que tenga en cuenta los llamados recursivos y que los cuente. Creo que con eso podrían pasar los tests que quedaron comentados. Me imagino algo como: `within ${toEDLString(declaration)} count(calls) + count(calls ${toEDLString(declaration)})` |
asanzo
reviewed
Apr 11, 2022
asanzo
reviewed
Apr 11, 2022
app/utils/expectations.js
Outdated
Comment on lines
33
to
35
| const countWithRecursiveCalling = (declaration) => | ||
| `within ${toEDLString(declaration)} count(calls) + count(calls ${toEDLString(declaration)})` | ||
|
|
There was a problem hiding this comment.
Debería llamarse countCallsWithin y adentro tener un comentario que explique este problema
There was a problem hiding this comment.
Cargar pregunta/issue en mulang ¿por qué el count(calls) no incluye las recursivas?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Al final nos dimos cuenta que el problema no era el AST que estabamos generando sino que la expectativa de doesNotUseRecursion.
Por lo que entendemos, esta expectativa:
through `A` ! calls `A`En este workspace:
El
throughcheckea que exista algun scope transitivo al procedimientoAdonde ocurra que no se llama aA. Esto ocurre en el procedimientoA, donde no se esta llamando aA, por lo que la expectativa pasa.Por esto, al poner el not abarcando todo el
throughestamos checkeando lo que queremos, que es que nunca desde el procedimiento se llegue a llamar a A.Respecto al otro caso, que era que las llamadas recursivas no estaban siendo contadas como hacer algo, esto parece ser algo propio de Mulang porque al hacer, por ejemplo,
within `A` count(calls `A`) >= 1, mulang ahi si detecta las llamadas a del procedimiento a si mismo, pero al hacerwithin `A` count(calls) >= 1no.