Skip to content

Fix the resolveName function #971

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

Merged
merged 2 commits into from
Oct 27, 2014
Merged

Fix the resolveName function #971

merged 2 commits into from
Oct 27, 2014

Conversation

ahejlsberg
Copy link
Member

The resolveName function requires a replacement argument string which in most invocations is provided by calling identifierToString. That in turn causes a lot of upfront work to be performed just in case an error needs to be reported. That doesn't make sense, particularly for such a performance critical function.

This PR changes the nameArg argument to be a string | Identifier such that the identifierToString call can be deferred to when it is actually needed. It also gets rid of a nested function that was added to perform certain checks before returning the result (which in turn created a nested function closure on every invocation). These checks are now performed at the end of the function itself.

@@ -379,7 +348,7 @@ module ts {
if (ctor && ctor.locals) {
if (getSymbol(ctor.locals, name, meaning & SymbolFlags.Value)) {
// save the property node - later it will be used by 'returnResolvedSymbol' to report appropriate error
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this comment is out of date

@vladima
Copy link
Contributor

vladima commented Oct 27, 2014

👍

1 similar comment
@CyrusNajmabadi
Copy link
Contributor

👍

ahejlsberg added a commit that referenced this pull request Oct 27, 2014
@ahejlsberg ahejlsberg merged commit 20392de into master Oct 27, 2014
@ahejlsberg ahejlsberg deleted the fixResolveName branch October 27, 2014 13:48
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants