[SR-11092] Disallow backticks for non keywords

[keith]

Previous ID	SR-11092
Radar	None
Original Reporter	@keith
Type	Improvement

Environment

Xcode 11.0 beta 3 (11M362v)

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Improvement
Assignee	None
Priority	Medium

md5: 10cfae3f651a2aca53b85e1d6ea2fc9d

Issue Description:

Currently if you have this code:

let `foo` = 5
print(`foo`)

It runs fine:

% swift /tmp/foo.swift
5

I would expect this to produce a warning / error since "foo" isn't a keyword, and therefore the backticks aren't necessary.

This might be / have been an intentional allowance, but I found it surprising that you can wrap anything in them which implies to me when reading the code that it's wrapping a keyword.

[belkadan]

Hm. What do you think, @rintaro, @akyrtzi? It should at most be a warning, not an error, in case a new keyword is introduced, or in case someone is already doing this.

[akyrtzi]

> It should at most be a warning, not an error, in case a new keyword is introduced, or in case someone is already doing this.

Agreed, warning seems fine.

[swift-ci]

Comment by Aasim Kandrikar (JIRA)

So I've been working on this issue and have the warning working when it is not a keyword defined in TokenKinds.def. However I've found that it does not include accessor keywords such as get, set, didSet and others. Should using get with backticks produce a waring since var get = 0 is legal swift.

[rintaro]

As I mentioned in the PR (#26415 (comment) this is not that simple. We have numerous usages of isContextualKeyword(StringRef). At those position, we distinguish non-escaped {{tok::identifier}}s from escaped one. So we cannot just suggest to remove escaping there.

[akyrtzi]

There's a benefit of not warning on this, you may have some tool doing code-generation, it would simplify things for such tools if they can freely add backticks without having to worry that a backtick may trigger a warning.

[swift-ci]

Comment by Aasim Kandrikar (JIRA)

I'm going to close the pull request so someone else can come up with a better solution. However, I also think there should be more discussion on the need for the feature as it looks like there is some stuff to discuss on what should be warned.