From 5d988ea3269cd2ba3b708c92c5d93742753263e8 Mon Sep 17 00:00:00 2001 From: Frank Tang Date: Wed, 9 Sep 2020 17:21:00 -0700 Subject: [PATCH] Use better error messages for dateStyle/timeStyle Bug: v8:10880 Change-Id: I7a9ba96e4b0c83565c4749101082c661e21d5ef1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2400598 Reviewed-by: Jakob Kummerow Reviewed-by: Frank Tang Commit-Queue: Frank Tang Cr-Commit-Position: refs/heads/master@{#69844} --- src/common/message-template.h | 1 + src/objects/js-date-time-format.cc | 33 +++++++++++++++---- .../PrivateAccessorAccess.golden | 8 ++--- .../PrivateMethodAccess.golden | 4 +-- .../StaticPrivateMethodAccess.golden | 20 +++++------ test/intl/regress-10438.js | 4 +-- test/intl/regress-10613.js | 5 +-- 7 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/common/message-template.h b/src/common/message-template.h index 675c9cbe10..d8f10d098f 100644 --- a/src/common/message-template.h +++ b/src/common/message-template.h @@ -315,6 +315,7 @@ namespace internal { T(BigIntDivZero, "Division by zero") \ T(BigIntNegativeExponent, "Exponent must be positive") \ T(BigIntTooBig, "Maximum BigInt size exceeded") \ + T(CantSetOptionXWhenYIsUsed, "Can't set option % when % is used") \ T(DateRange, "Provided date is not in valid range.") \ T(ExpectedLocation, \ "Expected letters optionally connected with underscores or hyphens for " \ diff --git a/src/objects/js-date-time-format.cc b/src/objects/js-date-time-format.cc index b936108aa6..310c97cfb0 100644 --- a/src/objects/js-date-time-format.cc +++ b/src/objects/js-date-time-format.cc @@ -1744,13 +1744,32 @@ MaybeHandle JSDateTimeFormat::New( // iii. If p is not undefined, then // 1. Throw a TypeError exception. if (skeleton.length() > 0) { - THROW_NEW_ERROR(isolate, - NewTypeError(MessageTemplate::kInvalid, - factory->NewStringFromStaticChars("option"), - date_style != DateTimeStyle::kUndefined - ? factory->dateStyle_string() - : factory->timeStyle_string()), - JSDateTimeFormat); + std::string prop; + for (const auto& item : GetPatternItems()) { + for (const auto& pair : item.pairs) { + if (skeleton.find(pair.pattern) != std::string::npos) { + prop.assign(item.property); + break; + } + } + if (!prop.empty()) { + break; + } + } + if (prop.empty() && skeleton.find("S") != std::string::npos) { + prop.assign("fractionalSecondDigits"); + } + if (!prop.empty()) { + THROW_NEW_ERROR( + isolate, + NewTypeError(MessageTemplate::kCantSetOptionXWhenYIsUsed, + factory->NewStringFromAsciiChecked(prop.c_str()), + date_style != DateTimeStyle::kUndefined + ? factory->dateStyle_string() + : factory->timeStyle_string()), + JSDateTimeFormat); + } + UNREACHABLE(); } // b. Let pattern be DateTimeStylePattern(dateStyle, timeStyle, // dataLocaleData, hc). diff --git a/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden b/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden index da6b0a2b26..78b08a41f8 100644 --- a/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden +++ b/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden @@ -84,7 +84,7 @@ bytecodes: [ B(Mov), R(this), R(0), B(Mov), R(context), R(2), /* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), - /* 53 S> */ B(Wide), B(LdaSmi), I16(266), + /* 53 S> */ B(Wide), B(LdaSmi), I16(267), B(Star), R(3), B(LdaConstant), U8(0), B(Star), R(4), @@ -115,7 +115,7 @@ bytecodes: [ B(Mov), R(this), R(0), B(Mov), R(context), R(2), /* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), - /* 46 S> */ B(Wide), B(LdaSmi), I16(265), + /* 46 S> */ B(Wide), B(LdaSmi), I16(266), B(Star), R(3), B(LdaConstant), U8(0), B(Star), R(4), @@ -146,7 +146,7 @@ bytecodes: [ B(Mov), R(this), R(0), B(Mov), R(context), R(2), /* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), - /* 53 S> */ B(Wide), B(LdaSmi), I16(266), + /* 53 S> */ B(Wide), B(LdaSmi), I16(267), B(Star), R(3), B(LdaConstant), U8(0), B(Star), R(4), @@ -177,7 +177,7 @@ bytecodes: [ B(Mov), R(this), R(0), B(Mov), R(context), R(2), /* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), - /* 46 S> */ B(Wide), B(LdaSmi), I16(265), + /* 46 S> */ B(Wide), B(LdaSmi), I16(266), B(Star), R(4), B(LdaConstant), U8(0), B(Star), R(5), diff --git a/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden b/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden index 9c278a2f54..8413da93a8 100644 --- a/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden +++ b/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden @@ -57,7 +57,7 @@ bytecodes: [ B(Mov), R(this), R(0), B(Mov), R(context), R(2), /* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), - /* 49 S> */ B(Wide), B(LdaSmi), I16(264), + /* 49 S> */ B(Wide), B(LdaSmi), I16(265), B(Star), R(3), B(LdaConstant), U8(0), B(Star), R(4), @@ -89,7 +89,7 @@ bytecodes: [ B(Mov), R(this), R(0), B(Mov), R(context), R(2), /* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), - /* 49 S> */ B(Wide), B(LdaSmi), I16(264), + /* 49 S> */ B(Wide), B(LdaSmi), I16(265), B(Star), R(3), B(LdaConstant), U8(0), B(Star), R(4), diff --git a/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden b/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden index 5ce6e397b7..6d5bb4a58f 100644 --- a/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden +++ b/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden @@ -25,7 +25,7 @@ bytecodes: [ B(TestReferenceEqual), R(this), B(Mov), R(this), R(1), B(JumpIfTrue), U8(18), - B(Wide), B(LdaSmi), I16(262), + B(Wide), B(LdaSmi), I16(263), B(Star), R(2), B(LdaConstant), U8(0), B(Star), R(3), @@ -56,7 +56,7 @@ frame size: 2 parameter count: 1 bytecode array length: 16 bytecodes: [ - /* 56 S> */ B(Wide), B(LdaSmi), I16(264), + /* 56 S> */ B(Wide), B(LdaSmi), I16(265), B(Star), R(0), B(LdaConstant), U8(0), B(Star), R(1), @@ -83,7 +83,7 @@ frame size: 2 parameter count: 1 bytecode array length: 16 bytecodes: [ - /* 56 S> */ B(Wide), B(LdaSmi), I16(264), + /* 56 S> */ B(Wide), B(LdaSmi), I16(265), B(Star), R(0), B(LdaConstant), U8(0), B(Star), R(1), @@ -122,7 +122,7 @@ bytecodes: [ /* 94 E> */ B(TestReferenceEqual), R(this), B(Mov), R(this), R(0), B(JumpIfTrue), U8(18), - B(Wide), B(LdaSmi), I16(262), + B(Wide), B(LdaSmi), I16(263), B(Star), R(2), B(LdaConstant), U8(0), B(Star), R(3), @@ -144,7 +144,7 @@ bytecodes: [ /* 109 E> */ B(TestReferenceEqual), R(this), B(Mov), R(this), R(1), B(JumpIfTrue), U8(18), - B(Wide), B(LdaSmi), I16(263), + B(Wide), B(LdaSmi), I16(264), B(Star), R(3), B(LdaConstant), U8(0), B(Star), R(4), @@ -159,7 +159,7 @@ bytecodes: [ /* 133 E> */ B(TestReferenceEqual), R(this), B(Mov), R(this), R(0), B(JumpIfTrue), U8(18), - B(Wide), B(LdaSmi), I16(262), + B(Wide), B(LdaSmi), I16(263), B(Star), R(2), B(LdaConstant), U8(0), B(Star), R(3), @@ -189,7 +189,7 @@ frame size: 2 parameter count: 1 bytecode array length: 16 bytecodes: [ - /* 60 S> */ B(Wide), B(LdaSmi), I16(266), + /* 60 S> */ B(Wide), B(LdaSmi), I16(267), B(Star), R(0), B(LdaConstant), U8(0), B(Star), R(1), @@ -215,7 +215,7 @@ frame size: 2 parameter count: 1 bytecode array length: 16 bytecodes: [ - /* 53 S> */ B(Wide), B(LdaSmi), I16(265), + /* 53 S> */ B(Wide), B(LdaSmi), I16(266), B(Star), R(0), B(LdaConstant), U8(0), B(Star), R(1), @@ -241,7 +241,7 @@ frame size: 2 parameter count: 1 bytecode array length: 16 bytecodes: [ - /* 60 S> */ B(Wide), B(LdaSmi), I16(266), + /* 60 S> */ B(Wide), B(LdaSmi), I16(267), B(Star), R(0), B(LdaConstant), U8(0), B(Star), R(1), @@ -267,7 +267,7 @@ frame size: 3 parameter count: 1 bytecode array length: 16 bytecodes: [ - /* 46 S> */ B(Wide), B(LdaSmi), I16(265), + /* 46 S> */ B(Wide), B(LdaSmi), I16(266), B(Star), R(1), B(LdaConstant), U8(0), B(Star), R(2), diff --git a/test/intl/regress-10438.js b/test/intl/regress-10438.js index 0aafe4d700..a1bdf36f70 100644 --- a/test/intl/regress-10438.js +++ b/test/intl/regress-10438.js @@ -44,10 +44,10 @@ assertThrows( () => (new Intl.DateTimeFormat( "en", {timeStyle: "short", fractionalSecondDigits: 3})), TypeError, - "Invalid option : timeStyle"); + "Can't set option fractionalSecondDigits when timeStyle is used"); assertThrows( () => (new Intl.DateTimeFormat( "en", {dateStyle: "short", fractionalSecondDigits: 3})), TypeError, - "Invalid option : dateStyle"); + "Can't set option fractionalSecondDigits when dateStyle is used"); diff --git a/test/intl/regress-10613.js b/test/intl/regress-10613.js index 1b15f3415c..55e04e9c91 100644 --- a/test/intl/regress-10613.js +++ b/test/intl/regress-10613.js @@ -14,6 +14,7 @@ let opt = { day: '2-digit', hour: '2-digit', minute: '2-digit', + fractionalSecondDigits: 2, }; let keys = Object.keys(opt); @@ -25,10 +26,10 @@ testTimeStyle.timeStyle = 'long'; for (key of keys) { assertThrows( () => new Intl.DateTimeFormat('en', testDateStyle), - TypeError, "Invalid option : dateStyle"); + TypeError, "Can't set option " + key + " when dateStyle is used"); assertThrows( () => new Intl.DateTimeFormat('en', testTimeStyle), - TypeError, "Invalid option : timeStyle"); + TypeError, "Can't set option " + key + " when timeStyle is used"); testDateStyle[key] = undefined; testTimeStyle[key] = undefined; }