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

disallow unrecognized string/char escapes #22800

Merged
merged 1 commit into from
Jul 14, 2017

Conversation

stevengj
Copy link
Member

As discussed in #21284, this PR disallows a backslash in string or character literals before unrecognized escape characters.

Besides the standard escapes that produce a different character (e.g. \n), the only allowed escapes where \* produces * are now for * ∈ {',",`,$,\}.

@stevengj stevengj added breaking This change will break code parser Language parsing and surface syntax labels Jul 13, 2017
@stevengj
Copy link
Member Author

(Note that I found a few bugs this way.)

@stevengj
Copy link
Member Author

I don't really understand the Travis failures. One appears to be a timeout, whereas the other is the mysterious:

	From worker 1:	compile: Test Failed
  Expression: Foo.g() === 97.0
   Evaluated: 2.0 === 97.0
Stacktrace:
 [1] (::Test62Main_compile.##1#14)() at /tmp/julia/share/julia/test/compile.jl:159

AppVeyor and FreeBSD and OSX Travis succeeded; is there some problem with Linux Travis now?

@vtjnash
Copy link
Member

vtjnash commented Jul 14, 2017

We've seen that one before. It seems to suggest we're losing a backedge at some point (stochastically), but we haven't reproduced it yet locally.

@JeffBezanson
Copy link
Member

Travis is failing in all kinds of ways lately.

@JeffBezanson JeffBezanson merged commit 7cb02cd into JuliaLang:master Jul 14, 2017
@stevengj stevengj deleted the invalidescapes branch July 14, 2017 13:13
@StefanKarpinski
Copy link
Member

The fact that this revealed a bunch of bugs is a pretty good indicator that this was a good idea.

@stevengj
Copy link
Member Author

There was a suggestion on discourse that this should be changed to a warning.

On the one hand, it seems like virtually all the errors that are popping up are bugs. On the other hand, we normally make changes like this a warning for a while...

@JeffBezanson
Copy link
Member

These are very easy to fix, and the fixed code is both backwards and forwards compatible, so it doesn't seem so crucial to me to give a warning.

@StefanKarpinski
Copy link
Member

Especially since when someone upgrades to 0.7, they should fix all warnings and then upgrade to 1.0, at which point this would fail anyway. There's even a case to be made that 0.7 should have depwarn=error on by default, but that's a different discussion.

samoconnor added a commit to JuliaCloud/AWSSDK.jl that referenced this pull request Aug 20, 2017
samoconnor added a commit to JuliaCloud/AWSCore.jl that referenced this pull request Aug 20, 2017
@ChrisRackauckas
Copy link
Member

ChrisRackauckas commented Apr 6, 2018

Did this cause math blocks to no longer work?

"""
    calculate_residuals!(out, ũ, u₀, u₁, α, ρ)

Save element-wise residuals
```math
\frac{ũ}{α+\max{|u₀|,|u₁|}*ρ}
```
in `out`.
"""
function calculate_residuals!(out, ũ, u₀, u₁, α, ρ, internalnorm)
   @. out = ũ / (α + max(internalnorm(u₀), internalnorm(u₁)) * ρ)
end
ERROR: syntax: invalid escape sequence

@KristofferC
Copy link
Member

Just escape the backslash?

@ChrisRackauckas
Copy link
Member

Having to go and escape the TeX code in every docstring is a pretty heavy handed workaround. For normal strings you'd just use LaTeXStrings.jl. Is there no simple way to write TeX in docstrings now? (Do string macros work on docstrings? I guess I never checked)

@stevengj
Copy link
Member Author

stevengj commented Apr 6, 2018

Note that \frac didn't work previously, either; it would have given frac since \f was equivalent to f. So, throwing an error instead caught a bug.

However, you can just use doc"..." to avoid the need for backslash escapes.

doc"""
    calculate_residuals!(out, ũ, u₀, u₁, α, ρ)

Save element-wise residuals
```math
\frac{ũ}{α+\max{|u₀|,|u₁|}*ρ}
```
in `out`.
"""

@stevengj
Copy link
Member Author

stevengj commented Apr 6, 2018

The manual has a brief comment about this:

@doc_str should only be used when the docstring contains $ or \ characters that should not be parsed by Julia such as LaTeX syntax or Julia source code examples containing interpolation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants