Skip to content

Commit 1e4421f

Browse files
steve-ssteve-s
committed
Formatting fixes: spec validation in int/string format and support %c in both traditional and printf style formatting
1 parent 7d5fb53 commit 1e4421f

File tree

6 files changed

+45
-102
lines changed

6 files changed

+45
-102
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_float.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
*graalpython.lib-python.3.test.test_float.ReprTestCase.test_repr
3636
*graalpython.lib-python.3.test.test_float.ReprTestCase.test_short_repr
3737
*graalpython.lib-python.3.test.test_float.RoundTestCase.test_None_ndigits
38+
*graalpython.lib-python.3.test.test_float.RoundTestCase.test_format_specials
3839
*graalpython.lib-python.3.test.test_float.RoundTestCase.test_inf_nan
3940
*graalpython.lib-python.3.test.test_float.RoundTestCase.test_large_n
4041
*graalpython.lib-python.3.test.test_float.RoundTestCase.test_matches_float_format

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/ints/IntBuiltins.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2630,8 +2630,12 @@ private static void validateIntegerSpec(PythonCore core, Spec spec) {
26302630
if (Spec.specified(spec.precision)) {
26312631
throw core.raise(ValueError, ErrorMessages.PRECISION_NOT_ALLOWED_FOR_INT);
26322632
}
2633-
if (spec.type == 'c' && Spec.specified(spec.sign)) {
2634-
throw core.raise(ValueError, ErrorMessages.SIGN_NOT_ALLOWED_WITH_C_FOR_INT);
2633+
if (spec.type == 'c') {
2634+
if (Spec.specified(spec.sign)) {
2635+
throw core.raise(ValueError, ErrorMessages.SIGN_NOT_ALLOWED_WITH_C_FOR_INT);
2636+
} else if (spec.alternate) {
2637+
throw core.raise(ValueError, ErrorMessages.ALTERNATE_NOT_ALLOWED_WITH_C_FOR_INT);
2638+
}
26352639
}
26362640
}
26372641
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/str/StringBuiltins.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,17 +180,30 @@ Object emptyFormat(VirtualFrame frame, Object self, @SuppressWarnings("unused")
180180
Object format(Object self, String formatString,
181181
@Cached CastToJavaStringCheckedNode castToJavaStringNode) {
182182
String str = castToJavaStringNode.cast(self, INVALID_RECEIVER, __STR__, self);
183-
return formatString(getCore(), formatString, str);
183+
return formatString(getCore(), getAndValidateSpec(formatString), str);
184184
}
185185

186186
@Fallback
187187
Object other(@SuppressWarnings("unused") Object self, Object formatString) {
188188
throw raise(PythonBuiltinClassType.TypeError, ErrorMessages.ARG_D_MUST_BE_S_NOT_P, "format()", 2, "str", formatString);
189189
}
190190

191+
private Spec getAndValidateSpec(String formatString) {
192+
Spec spec = InternalFormat.fromText(getCore(), formatString, __FORMAT__);
193+
if (Spec.specified(spec.type) && spec.type != 's') {
194+
throw raise(PythonBuiltinClassType.TypeError, ErrorMessages.UNKNOWN_FORMAT_CODE, spec.type, "str");
195+
}
196+
if (spec.alternate) {
197+
throw raise(PythonBuiltinClassType.ValueError, ErrorMessages.ALTERNATE_NOT_ALLOWED_WITH_STRING_FMT);
198+
}
199+
if (Spec.specified(spec.align) && spec.align == '=') {
200+
throw raise(PythonBuiltinClassType.ValueError, ErrorMessages.EQUALS_ALIGNMENT_FLAG_NOT_ALLOWED_FOR_STRING_FMT);
201+
}
202+
return spec;
203+
}
204+
191205
@TruffleBoundary
192-
private static Object formatString(PythonCore core, String formatString, String str) {
193-
Spec spec = InternalFormat.fromText(core, formatString, __FORMAT__);
206+
private static Object formatString(PythonCore core, Spec spec, String str) {
194207
TextFormatter formatter = new TextFormatter(core, spec.withDefaults(Spec.STRING));
195208
formatter.format(str);
196209
return formatter.pad().getResult();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,6 @@ public abstract class ErrorMessages {
381381
public static final String NON_STRING_IN_CODE_SLOT = "non-string found in code slot";
382382
public static final String NOT_A_ZIP_FILE = "not a Zip file: '%s'";
383383
public static final String NOT_ALL_ARGS_CONVERTED_DURING_FORMATTING = "not all arguments converted during %s formatting";
384-
public static final String NOT_ALLOWED_S_S_FORMAT_SPECIFIERS_S = "%s not allowed %s%s format specifier%s";
385384
public static final String NOT_ENOUGH_ARGS_FOR_FORMAT_STRING = "not enough arguments for format string";
386385
public static final String NOT_ENOUGH_VALUES_TO_UNPACK = "not enough values to unpack (expected %d, got %d)";
387386
public static final String NOT_SUPPORTED_BETWEEN_INSTANCES = "'%s' not supported between instances of '%p' and '%p'";
@@ -541,11 +540,14 @@ public abstract class ErrorMessages {
541540
public static final String POW_BASE_NOT_INVERTIBLE = "base is not invertible for the given modulus";
542541
public static final String POW_ZERO_CANNOT_RAISE_TO_NEGATIVE_POWER = "0.0 cannot be raised to a negative power";
543542
public static final String S_ALIGNMENT_FLAG_NOT_ALLOWED_FOR_COMPLEX_FMT = "'%c' alignment flag is not allowed in complex format specifier";
543+
public static final String EQUALS_ALIGNMENT_FLAG_NOT_ALLOWED_FOR_STRING_FMT = "'=' alignment not allowed in string format specifier";
544544
public static final String ZERO_PADDING_NOT_ALLOWED_FOR_COMPLEX_FMT = "Zero padding is not allowed in complex format specifier";
545545
public static final String POW_THIRD_ARG_CANNOT_BE_ZERO = "pow() 3rd argument cannot be 0";
546546
public static final String CANNOT_ENCODE_DOCSTR = "'utf-8' codec can't encode docstring '%s'";
547547
public static final String PRECISION_NOT_ALLOWED_FOR_INT = "Precision not allowed in integer format specifier";
548548
public static final String SIGN_NOT_ALLOWED_WITH_C_FOR_INT = "Sign not allowed with integer format specifier 'c'";
549+
public static final String ALTERNATE_NOT_ALLOWED_WITH_C_FOR_INT = "Alternate form (#) not allowed with integer format specifier 'c'";
550+
public static final String ALTERNATE_NOT_ALLOWED_WITH_STRING_FMT = "Alternate form (#) not allowed in string format specifier";
549551
public static final String CAPI_LOAD_ERROR = "Could not load C API from %s.";
550552
public static final String NATIVE_ACCESS_NOT_ALLOWED = "Cannot run any C extensions because native access is not allowed.";
551553
public static final String HPY_LOAD_ERROR = "Could not load HPy C API from %s.";

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/formatting/FormattingUtils.java

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,7 @@
4040
*/
4141
package com.oracle.graal.python.runtime.formatting;
4242

43-
import static com.oracle.graal.python.runtime.exception.PythonErrorType.ValueError;
44-
45-
import com.oracle.graal.python.nodes.ErrorMessages;
4643
import com.oracle.graal.python.runtime.PythonCore;
47-
import com.oracle.graal.python.runtime.PythonParser.ParserErrorCallback;
48-
import com.oracle.graal.python.runtime.exception.PException;
4944
import com.oracle.graal.python.runtime.formatting.InternalFormat.Formatter;
5045
import com.oracle.graal.python.runtime.formatting.InternalFormat.Spec;
5146

@@ -64,64 +59,11 @@ public static Spec validateAndPrepareForFloat(Spec spec, PythonCore core, String
6459
case 'F':
6560
case 'G':
6661
case '%':
67-
// Check for disallowed parts of the specification
68-
if (spec.alternate) {
69-
throw alternateFormNotAllowed(core, forType);
70-
}
7162
// spec may be incomplete. The defaults are those commonly used for numeric
7263
// formats.
7364
return spec.withDefaults(InternalFormat.Spec.NUMERIC);
7465
default:
7566
throw Formatter.unknownFormat(core, spec.type, forType);
7667
}
7768
}
78-
79-
/**
80-
* Convenience method returning a {ValueError} reporting that alternate form is not allowed in a
81-
* format specifier for the named type.
82-
*
83-
* @param forType the type it was found applied to
84-
* @return exception to throw
85-
*/
86-
public static PException alternateFormNotAllowed(ParserErrorCallback errors, String forType) {
87-
return alternateFormNotAllowed(errors, forType, '\0');
88-
}
89-
90-
/**
91-
* Convenience method returning a {ValueError} reporting that alternate form is not allowed in a
92-
* format specifier for the named type and specified typoe code.
93-
*
94-
* @param forType the type it was found applied to
95-
* @param code the formatting code (or '\0' not to mention one)
96-
* @return exception to throw
97-
*/
98-
public static PException alternateFormNotAllowed(ParserErrorCallback errors, String forType, char code) {
99-
return notAllowed(errors, "Alternate form (#)", forType, code);
100-
}
101-
102-
/**
103-
* Convenience method returning a {ValueError} reporting that some format specifier feature is
104-
* not allowed for the named format code and data type. Produces a message like:
105-
* <p>
106-
* <code>outrage+" not allowed with "+forType+" format specifier '"+code+"'"</code>
107-
* <p>
108-
* <code>outrage+" not allowed in "+forType+" format specifier"</code>
109-
*
110-
* @param outrage committed in the present case
111-
* @param forType the data type (e.g. "integer") it where it is an outrage
112-
* @param code the formatting code for which it is an outrage (or '\0' not to mention one)
113-
* @return exception to throw
114-
*/
115-
public static PException notAllowed(ParserErrorCallback errors, String outrage, String forType, char code) {
116-
// Try really hard to be like CPython
117-
String codeAsString, withOrIn;
118-
if (code == 0) {
119-
withOrIn = "in ";
120-
codeAsString = "";
121-
} else {
122-
withOrIn = "with ";
123-
codeAsString = " '" + code + "'";
124-
}
125-
throw errors.raise(ValueError, ErrorMessages.NOT_ALLOWED_S_S_FORMAT_SPECIFIERS_S, outrage, withOrIn, forType, codeAsString);
126-
}
12769
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/formatting/IntegerFormatter.java

Lines changed: 19 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,16 @@ void format_b(BigInteger value) {
278278
}
279279

280280
/**
281-
* @see #format_c(int)
281+
* Format the value as a character (into {@link #result}).
282+
*
283+
* @param value to convert
282284
*/
283-
void format_c(@SuppressWarnings("unused") BigInteger value) {
284-
throw unknownFormat(errors, spec.type, "integer");
285+
final void format_c(BigInteger value) {
286+
assert !bytes; // for bytes we use directly BytesFormatter
287+
if (value.signum() < 0 || value.compareTo(LIMIT_UNICODE) >= 0) {
288+
throw errors.raise(OverflowError, ErrorMessages.C_ARG_NOT_IN_RANGE, toHexString(LIMIT_UNICODE));
289+
}
290+
result.appendCodePoint(value.intValue());
285291
}
286292

287293
// Limits used in format_c(BigInteger)
@@ -347,8 +353,7 @@ public IntegerFormatter format(int value) {
347353
break;
348354

349355
default:
350-
// Should never get here, since this was checked in caller.
351-
throw unknownFormat(errors, spec.type, "integer");
356+
throw unknownFormat(errors, spec.type, "int");
352357
}
353358

354359
// If required to, group the whole-part digits.
@@ -453,12 +458,16 @@ void format_b(int value) {
453458
}
454459

455460
/**
456-
* Format the value as a character (into {@link #result}). Note: 'c' format is not supported in
457-
* format builtin, but supported in the printf-style formatting. This method is overridden by
458-
* the {@link Traditional} subclass.
461+
* Format the value as a character (into {@link #result}).
462+
*
463+
* @param value to convert
459464
*/
460-
void format_c(@SuppressWarnings("unused") int value) {
461-
throw unknownFormat(errors, spec.type, "integer");
465+
final void format_c(int value) {
466+
assert !bytes; // for bytes we use directly BytesFormatter
467+
if (value < 0 || value >= LIMIT_UNICODE.intValue()) {
468+
throw errors.raise(OverflowError, ErrorMessages.C_ARG_NOT_IN_RANGE, toHexString(LIMIT_UNICODE));
469+
}
470+
result.appendCodePoint(value);
462471
}
463472

464473
/**
@@ -640,34 +649,6 @@ public Traditional(PythonCore core, FormattingBuffer result, Spec spec) {
640649
super(core, result, spec);
641650
}
642651

643-
/**
644-
* Format the value as a character (into {@link #result}).
645-
*
646-
* @param value to convert
647-
*/
648-
@Override
649-
void format_c(BigInteger value) {
650-
assert !bytes; // for bytes we use directly BytesFormatter
651-
if (value.signum() < 0 || value.compareTo(LIMIT_UNICODE) >= 0) {
652-
throw errors.raise(OverflowError, ErrorMessages.C_ARG_NOT_IN_RANGE, toHexString(LIMIT_UNICODE));
653-
}
654-
result.appendCodePoint(value.intValue());
655-
}
656-
657-
/**
658-
* Format the value as a character (into {@link #result}).
659-
*
660-
* @param value to convert
661-
*/
662-
@Override
663-
void format_c(int value) {
664-
assert !bytes; // for bytes we use directly BytesFormatter
665-
if (value < 0 || value >= LIMIT_UNICODE.intValue()) {
666-
throw errors.raise(OverflowError, ErrorMessages.C_ARG_NOT_IN_RANGE, toHexString(LIMIT_UNICODE));
667-
}
668-
result.appendCodePoint(value);
669-
}
670-
671652
@Override
672653
void format_i(BigInteger value) {
673654
format_n(value);

0 commit comments

Comments
 (0)