Skip to content

Error on redeclarations of undefined#5420

Merged
weswigham merged 6 commits into
microsoft:masterfrom
weswigham:undefined-unique
Dec 5, 2015
Merged

Error on redeclarations of undefined#5420
weswigham merged 6 commits into
microsoft:masterfrom
weswigham:undefined-unique

Conversation

@weswigham

Copy link
Copy Markdown
Member

From discussion on #5387.

Instead of this variety of implementation, another other option is actually exposing the undefined type as a primitive-like type (like symbol or number) and adding declare var undefined: undefined to the lib. That would be more consistent - and could begin to pave the way to more easily explicitly "nullable" types.

Fixes #3087.

Comment thread src/compiler/diagnosticMessages.json Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just curious, why not use Duplicate identifier '{0}' error?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd prefer a more helpful message. This is the better direction IMO.

Though, :%s/duplicates/name conflicts :%s/builtin/built-in. P.S. I don't know if those are valid vim replacement commands.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am fine with the direction. Just curious and I do like the new error :)

Comment thread src/compiler/checker.ts Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so why not just let them merge, then later on make sure that the undefined symbol is the one we have created, or that it does not have any declarations?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can get some wonky undefined behavior while typechecking if we actually let them merge (eg, the creation of a class which is also undefined), so we have to guard against it in the merge function so we don't let it poison the type checking process. Outright overriding the symbol is okay (since then we still have the real undefined type symbol to use internally) and is what #5387 allowed, but merging with the builtin undefined symbol is unallowable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a corner case, if the user ran into it, we just should let them know, i do not think we need to complicate our response here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

By "wonky undefined behavior" I mean crashing/exceptions.

@yuit

yuit commented Dec 5, 2015

Copy link
Copy Markdown
Contributor

👍 🍸

weswigham added a commit that referenced this pull request Dec 5, 2015
Error on redeclarations of undefined
@weswigham
weswigham merged commit 4735c00 into microsoft:master Dec 5, 2015
@weswigham
weswigham deleted the undefined-unique branch October 12, 2017 08:05
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

6 participants