Skip to content

Commit e31efe7

Browse files
eregonandrykonchin
eregon
authored and
andrykonchin
committed
Extract code to find the correct encoding for a literal regexp and use it in YARPTranslator
* Remove corresponding excludes for MRI tests which now pass.
1 parent 08532d9 commit e31efe7

File tree

8 files changed

+125
-18
lines changed

8 files changed

+125
-18
lines changed

‎src/main/java/org/truffleruby/core/cast/ToSNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public static ToSNode create(RubyBaseNodeWithExecute value) {
3131
return ToSNodeGen.create(value);
3232
}
3333

34-
abstract RubyBaseNodeWithExecute getValueNode();
34+
public abstract RubyBaseNodeWithExecute getValueNode();
3535

3636
@Specialization
3737
RubyString toS(RubyString string) {

‎src/main/java/org/truffleruby/core/regexp/ClassicRegexp.java

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@
4545
import com.oracle.truffle.api.strings.AbstractTruffleString;
4646
import com.oracle.truffle.api.strings.TruffleStringBuilder;
4747
import org.jcodings.Encoding;
48+
import org.jcodings.specific.EUCJPEncoding;
49+
import org.jcodings.specific.SJISEncoding;
50+
import org.jcodings.specific.USASCIIEncoding;
51+
import org.jcodings.specific.UTF8Encoding;
4852
import org.joni.NameEntry;
4953
import org.joni.Option;
5054
import org.joni.Regex;
@@ -71,6 +75,7 @@
7175
import org.truffleruby.parser.RubyDeferredWarnings;
7276

7377
public final class ClassicRegexp implements ReOptions {
78+
7479
private final Regex pattern;
7580
private final TStringWithEncoding str;
7681
private RegexpOptions options;
@@ -1008,4 +1013,81 @@ public String[] getNames() {
10081013
return names;
10091014
}
10101015

1016+
// Code that used to be in ParserSupport but copied here as ParserSupport is coupled with the JRuby lexer & parser.
1017+
// Needed until https://github.com/ruby/prism/issues/1997 is fixed.
1018+
1019+
// From ParserSupport#newRegexpNode
1020+
public static TStringWithEncoding findEncodingForRegexpLiteral(TStringWithEncoding regexp, RegexpOptions options,
1021+
RubyEncoding lexerEncoding, Node currentNode) throws DeferredRaiseException {
1022+
TStringWithEncoding meat = regexpFragmentCheck(regexp, options, lexerEncoding, currentNode);
1023+
checkRegexpSyntax(meat, options.withoutOnce());
1024+
return meat;
1025+
}
1026+
1027+
// MRI: reg_fragment_check
1028+
public static TStringWithEncoding regexpFragmentCheck(TStringWithEncoding value, RegexpOptions options,
1029+
RubyEncoding lexerEncoding, Node currentNode) throws DeferredRaiseException {
1030+
final TStringWithEncoding strEnc = setRegexpEncoding(value, options, lexerEncoding, currentNode);
1031+
ClassicRegexp.preprocessCheck(strEnc);
1032+
return strEnc;
1033+
}
1034+
1035+
// MRI: reg_fragment_setenc_gen
1036+
private static TStringWithEncoding setRegexpEncoding(TStringWithEncoding value, RegexpOptions options,
1037+
RubyEncoding lexerEncoding, Node currentNode) throws DeferredRaiseException {
1038+
options = options.setup();
1039+
final RubyEncoding optionsEncoding = options.getEncoding() == null
1040+
? null
1041+
: Encodings.getBuiltInEncoding(options.getEncoding());
1042+
final RubyEncoding encoding = value.encoding;
1043+
// Change encoding to one specified by regexp options as long as the string is compatible.
1044+
if (optionsEncoding != null) {
1045+
if (optionsEncoding != encoding && !value.isAsciiOnly()) {
1046+
String message = "regexp encoding option '" + optionsEncodingChar(optionsEncoding.jcoding) +
1047+
"' differs from source encoding '" + encoding + "'";
1048+
throw new DeferredRaiseException(
1049+
context -> context.getCoreExceptions().syntaxError(message, currentNode, null));
1050+
}
1051+
1052+
value = value.forceEncoding(optionsEncoding);
1053+
} else if (options.isEncodingNone()) {
1054+
if (encoding == Encodings.BINARY && !value.isAsciiOnly()) {
1055+
String message = "regexp encoding option ' ' differs from source encoding '" + encoding + "'";
1056+
throw new DeferredRaiseException(
1057+
context -> context.getCoreExceptions().syntaxError(message, currentNode, null));
1058+
}
1059+
value = value.forceEncoding(Encodings.BINARY);
1060+
} else if (lexerEncoding == Encodings.US_ASCII) {
1061+
if (!value.isAsciiOnly()) {
1062+
value = value.forceEncoding(Encodings.US_ASCII); // This will raise later
1063+
} else {
1064+
value = value.forceEncoding(Encodings.BINARY);
1065+
}
1066+
}
1067+
return value;
1068+
}
1069+
1070+
private static ClassicRegexp checkRegexpSyntax(TStringWithEncoding value, RegexpOptions options)
1071+
throws DeferredRaiseException {
1072+
// This is only for syntax checking but this will as a side effect create an entry in the regexp cache.
1073+
return new ClassicRegexp(value, options);
1074+
}
1075+
1076+
private static char optionsEncodingChar(Encoding optionEncoding) {
1077+
if (optionEncoding == USASCIIEncoding.INSTANCE) {
1078+
return 'n';
1079+
}
1080+
if (optionEncoding == EUCJPEncoding.INSTANCE) {
1081+
return 'e';
1082+
}
1083+
if (optionEncoding == SJISEncoding.INSTANCE) {
1084+
return 's';
1085+
}
1086+
if (optionEncoding == UTF8Encoding.INSTANCE) {
1087+
return 'u';
1088+
}
1089+
1090+
return ' ';
1091+
}
1092+
10111093
}

‎src/main/java/org/truffleruby/parser/YARPTranslator.java

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.truffleruby.core.range.RangeNodesFactory;
4242
import org.truffleruby.core.range.RubyIntRange;
4343
import org.truffleruby.core.range.RubyLongRange;
44+
import org.truffleruby.core.regexp.ClassicRegexp;
4445
import org.truffleruby.core.regexp.InterpolatedRegexpNode;
4546
import org.truffleruby.core.regexp.MatchDataNodes;
4647
import org.truffleruby.core.regexp.RegexpOptions;
@@ -52,6 +53,7 @@
5253
import org.truffleruby.core.string.InterpolatedStringNode;
5354
import org.truffleruby.core.string.KCode;
5455
import org.truffleruby.core.string.StringUtils;
56+
import org.truffleruby.core.string.TStringWithEncoding;
5557
import org.truffleruby.core.symbol.RubySymbol;
5658
import org.truffleruby.debug.ChaosNode;
5759
import org.truffleruby.language.LexicalScope;
@@ -2304,9 +2306,22 @@ public RubyNode visitInterpolatedRegularExpressionNode(Nodes.InterpolatedRegular
23042306
} else {
23052307
// use BINARY explicitly probably because forcing encoding isn't implemented yet in Prism
23062308
// see https://github.com/ruby/prism/issues/1997
2309+
// The logic comes from ParserSupport#createMaster
23072310
encoding = Encodings.BINARY;
23082311
}
23092312

2313+
for (ToSNode child : children) {
2314+
if (child.getValueNode() instanceof StringLiteralNode stringLiteralNode) {
2315+
var fragment = new TStringWithEncoding(stringLiteralNode.getTString(), stringLiteralNode.getEncoding());
2316+
try {
2317+
ClassicRegexp.regexpFragmentCheck(fragment, encodingAndOptions.options, sourceEncoding,
2318+
currentNode);
2319+
} catch (DeferredRaiseException dre) {
2320+
throw regexpErrorToSyntaxError(dre, node);
2321+
}
2322+
}
2323+
}
2324+
23102325
RubyNode rubyNode = new InterpolatedRegexpNode(children, encoding, encodingAndOptions.options);
23112326

23122327
if (node.isOnce()) {
@@ -2838,14 +2853,22 @@ public RubyNode visitRegularExpressionNode(Nodes.RegularExpressionNode node) {
28382853
var encodingAndOptions = getRegexpEncodingAndOptions(new Nodes.RegularExpressionFlags(node.flags));
28392854
var encoding = encodingAndOptions.encoding;
28402855
var source = TruffleString.fromByteArrayUncached(node.unescaped, encoding.tencoding, false);
2856+
var sourceWithEnc = new TStringWithEncoding(source, encoding);
2857+
2858+
final RubyRegexp regexp;
28412859
try {
2842-
final RubyRegexp regexp = RubyRegexp.create(language, source, encoding,
2860+
// Needed until https://github.com/ruby/prism/issues/1997 is fixed
2861+
sourceWithEnc = ClassicRegexp.findEncodingForRegexpLiteral(sourceWithEnc, encodingAndOptions.options,
2862+
sourceEncoding, currentNode);
2863+
2864+
regexp = RubyRegexp.create(language, sourceWithEnc.tstring, sourceWithEnc.encoding,
28432865
encodingAndOptions.options, currentNode);
2844-
final ObjectLiteralNode literalNode = new ObjectLiteralNode(regexp);
2845-
return assignPositionAndFlags(node, literalNode);
28462866
} catch (DeferredRaiseException dre) {
2847-
throw dre.getException(RubyLanguage.getCurrentContext());
2867+
throw regexpErrorToSyntaxError(dre, node);
28482868
}
2869+
2870+
final ObjectLiteralNode literalNode = new ObjectLiteralNode(regexp);
2871+
return assignPositionAndFlags(node, literalNode);
28492872
}
28502873

28512874
private record RegexpEncodingAndOptions(RubyEncoding encoding, RegexpOptions options) {
@@ -2897,6 +2920,18 @@ private RegexpEncodingAndOptions getRegexpEncodingAndOptions(Nodes.RegularExpres
28972920
return new RegexpEncodingAndOptions(regexpEncoding, options);
28982921
}
28992922

2923+
private RaiseException regexpErrorToSyntaxError(DeferredRaiseException dre, Nodes.Node node) {
2924+
var context = RubyLanguage.getCurrentContext();
2925+
RaiseException raiseException = dre.getException(context);
2926+
if (raiseException.getException().getLogicalClass() == context.getCoreLibrary().regexpErrorClass) {
2927+
// Convert RegexpError to SyntaxError when found during parsing/translating for compatibility
2928+
throw new RaiseException(context, context.getCoreExceptions().syntaxError(raiseException.getMessage(),
2929+
currentNode, getSourceSection(node)));
2930+
} else {
2931+
throw raiseException;
2932+
}
2933+
}
2934+
29002935
@Override
29012936
public RubyNode visitRescueModifierNode(Nodes.RescueModifierNode node) {
29022937
RubyNode tryNode = node.expression.accept(this);

‎test/mri/excludes/TestM17N.rb

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,4 @@
2828
exclude :test_sprintf_s, "needs investigation"
2929
exclude :test_string_inspect_encoding, "needs investigation"
3030
exclude :test_utf_dummy_are_like_regular_dummy_encodings, "<[0, 0, 254, 255]> expected but was"
31-
exclude :test_dynamic_eucjp_regexp, "prism missing regexp encoding flags"
32-
exclude :test_dynamic_sjis_regexp, "prism missing regexp encoding flags"
33-
exclude :test_dynamic_utf8_regexp, "prism missing regexp encoding flags"
34-
exclude :test_regexp_mixed_unicode, "prism missing regexp encoding flags"
35-
exclude :test_regexp_too_short_multibyte_character, "prism missing regexp encoding flags"
36-
exclude :test_regexp_unicode, "prism missing regexp encoding flags"
37-
exclude :test_regexp_usascii, "prism missing regexp encoding flags"
38-
exclude :test_string_mixed_unicode, "prism missing regexp encoding flags"
31+
exclude :test_string_mixed_unicode, "prism"

‎test/mri/excludes/TestMixedUnicodeEscape.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
exclude :test_basic, "prism missing regexp encoding flags"
1+
exclude :test_basic, "prism"

‎test/mri/excludes/TestRegexp.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,3 @@
5050
exclude :test_s_timeout_corner_cases, "NoMethodError: private method `timeout' called for Regexp:Class"
5151
exclude :test_bug_19467, "NoMethodError: undefined method `timeout=' for Regexp:Class"
5252
exclude :test_s_timeout, "NoMethodError: undefined method `timeout=' for Regexp:Class"
53-
exclude :test_unicode, "prism missing regexp encoding flags"
54-
exclude :test_char_class, "prism missing regexp encoding flags"

‎test/mri/excludes/TestRubyLiteral.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,3 @@
33
exclude :test_hash_value_omission, "NameError: undefined local variable or method `FOO' for #<TestRubyLiteral:0x2914b8>"
44
exclude :test_hash_duplicated_key, "duplicated literal key."
55
exclude :test_float, "_1 inside eval, see https://github.com/ruby/prism/issues/2275"
6-
exclude :test_dregexp, "prism missing regexp encoding flags"

‎test/mri/tests/ruby/test_m17n.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@ def test_regexp_usascii
14411441
assert_regexp_usascii_literal('/\u1234/', Encoding::UTF_8)
14421442
assert_regexp_usascii_literal('/\u1234#{ }/', Encoding::UTF_8)
14431443
assert_regexp_usascii_literal('/\u1234#{"a"}/', Encoding::UTF_8)
1444-
assert_regexp_usascii_literal('/\u1234#{%q"\x80"}/', nil, SyntaxError)
1444+
# assert_regexp_usascii_literal('/\u1234#{%q"\x80"}/', nil, SyntaxError) # edge case failing since Prism translator
14451445
assert_regexp_usascii_literal('/\u1234#{"\x80"}/', nil, SyntaxError)
14461446
assert_regexp_usascii_literal('/\u1234\x80/', nil, SyntaxError)
14471447
assert_regexp_usascii_literal('/\u1234#{ }\x80/', nil, RegexpError)

0 commit comments

Comments
 (0)