Skip to content

Commit cd3b454

Browse files
Deprecate single-letter macro fresh variables with indices (#16267)
Crystal supports 6 sigils which modify the meaning of string literals. Curly braces may be used instead of the usual double quotation marks for those strings: ```crystal %i{1} %q{2} %Q{3} %r{4} %x{5} %w{6} ``` The same syntax is used for fresh variables inside macros: ```crystal macro foo {% for i in 1..6 %} %b{i} = {{ i }} {% end %} {% for i in 1..6 %} p %b{i} {% end %} end foo ``` It is a syntax error here to use a fresh variable name that coincides with one of the sigils, because the sigil has precedence and assignment to a string literal is never allowed. This is problematic since adding _any_ new sigil to the language, e.g. `%b` for #2886, immediately introduces a breaking change by making those macros no longer valid. I believe the language should be more future-proof here, unless we can say for sure that new sigils must not be introduced. Co-authored-by: fn ⌃ ⌥ <[email protected]>
1 parent 314e646 commit cd3b454

File tree

3 files changed

+40
-3
lines changed

3 files changed

+40
-3
lines changed

spec/compiler/parser/warnings_spec.cr

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ private def assert_no_parser_warning(source, *, file = __FILE__, line = __LINE__
1919
warnings.should eq([] of String), file: file, line: line
2020
end
2121

22+
VALID_SIGILS = ['i', 'q', 'r', 'w', 'x', 'Q']
23+
2224
describe "Parser warnings" do
2325
it "warns on suffix-less UInt64 literals > Int64::MAX" do
2426
values = [
@@ -64,4 +66,36 @@ describe "Parser warnings" do
6466
assert_no_parser_warning("def foo : Foo\nend")
6567
end
6668
end
69+
70+
it "warns on single-letter macro fresh variables with indices" do
71+
chars = ('A'..'Z').to_a + ('a'..'z').to_a - VALID_SIGILS
72+
chars.each do |letter|
73+
assert_parser_warning <<-CRYSTAL, "Warning: single-letter macro fresh variables with indices are deprecated"
74+
macro foo
75+
%#{letter}{1} = 2
76+
end
77+
CRYSTAL
78+
end
79+
end
80+
81+
it "doesn't warn on sigils that resemble single-letter macro fresh variables with indices" do
82+
VALID_SIGILS.each do |letter|
83+
assert_no_parser_warning <<-CRYSTAL
84+
macro foo
85+
%#{letter}{1}
86+
end
87+
CRYSTAL
88+
end
89+
end
90+
91+
it "doesn't warn on single-letter macro fresh variables without indices" do
92+
('A'..'Z').each do |letter|
93+
assert_no_parser_warning <<-CRYSTAL
94+
macro foo
95+
%#{letter} = 1
96+
%#{letter.downcase} = 2
97+
end
98+
CRYSTAL
99+
end
100+
end
67101
end

src/compiler/crystal/syntax/parser.cr

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3281,6 +3281,9 @@ module Crystal
32813281
macro_var_name = @token.value.to_s
32823282
location = @token.location
32833283
if current_char == '{'
3284+
if macro_var_name.size == 1
3285+
warnings.add_warning_at @token.location, "single-letter macro fresh variables with indices are deprecated"
3286+
end
32843287
macro_var_exps = parse_macro_var_exps
32853288
else
32863289
macro_var_exps = nil

src/iterator.cr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,13 +1457,13 @@ module Iterator(T)
14571457
def next
14581458
{% begin %}
14591459
{% for i in 0...Is.size %}
1460-
%v{i} = @iterators[{{ i }}].next
1461-
return stop if %v{i}.is_a?(Stop)
1460+
%value{i} = @iterators[{{ i }}].next
1461+
return stop if %value{i}.is_a?(Stop)
14621462
{% end %}
14631463

14641464
Tuple.new(
14651465
{% for i in 0...Is.size %}
1466-
%v{i},
1466+
%value{i},
14671467
{% end %}
14681468
)
14691469
{% end %}

0 commit comments

Comments
 (0)