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

Normative: Make HasCallInTailPosition for ImportCall always return false #3106

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Jun 26, 2023

HasCallInTailPosition is currently implicitly defined on ImportCall via the chain rule: HasCallInTailPosition of import( xxx ) is HasCallInTailPosition of xxx. This is wrong, because xxx is not in tail position.

This PR explicitly defines it to be false.

For what is worth, JSC matches this PR (we can't talk about "web reality" in this case, it's just a spec bug):
image

@jmdyck
Copy link
Collaborator

jmdyck commented Jun 26, 2023

ImportCall has a single RHS, and that has a single non-terminal (AssignmentExpression), so the chain rule handles it. (HasCallInTailPosition is explicitly defined on AssignmentExpression.)

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Jun 26, 2023

Applying the chain rule would return true for foo() in this case, but it's not a tail call:

function fn() {
  return import(foo());
}

@jmdyck
Copy link
Collaborator

jmdyck commented Jun 26, 2023

Ah, so the problem is not that HasCallInTailPosition isn't defined for ImportCall, but rather that (via the chain rule) it's defined incorrectly?

@nicolo-ribaudo nicolo-ribaudo changed the title Define HasCallInTailPosition for ImportCall Fix HasCallInTailPosition for ImportCall Jun 26, 2023
@nicolo-ribaudo
Copy link
Member Author

I edited the PR description. My original flow of thoughts was "why isn't this defined to be false", but I was thinking more about the part about it not being explicitly defined 😅

Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

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

LGTM.

This should be landed as normative, but does not require plenary approval because it clearly fixes a bug so that it will match committee intent.

@michaelficarra michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jun 28, 2023
@nicolo-ribaudo nicolo-ribaudo changed the title Fix HasCallInTailPosition for ImportCall Normative: Make HasCallInTailPosition for ImportCall always return false Jun 29, 2023
@nicolo-ribaudo nicolo-ribaudo added the normative change Affects behavior required to correctly evaluate some ECMAScript source text label Jun 29, 2023
ljharb pushed a commit to nicolo-ribaudo/ecma262 that referenced this pull request Jun 29, 2023
@ljharb ljharb force-pushed the import-call-HasCallInTailPosition branch from 3470545 to c88e81e Compare June 29, 2023 17:04
@nicolo-ribaudo
Copy link
Member Author

@ljharb I updated the PR title to be more descriptive, if you want to use it as the commit message

@ljharb ljharb force-pushed the import-call-HasCallInTailPosition branch from c88e81e to 53454a9 Compare June 29, 2023 17:11
@ljharb ljharb merged commit 53454a9 into tc39:main Jun 29, 2023
6 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the import-call-HasCallInTailPosition branch June 29, 2023 17:40
zhangenming pushed a commit to zhangenming/ecma262 that referenced this pull request Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
normative change Affects behavior required to correctly evaluate some ECMAScript source text ready to merge Editors believe this PR needs no further reviews, and is ready to land. spec bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants