diff options
| -rw-r--r-- | js/src/frontend/NameAnalysisTypes.h | 12 | ||||
| -rw-r--r-- | js/src/frontend/Parser.cpp | 106 | ||||
| -rw-r--r-- | js/src/frontend/Parser.h | 16 | ||||
| -rw-r--r-- | js/src/jit-test/tests/parser/redeclaration.js | 230 | ||||
| -rw-r--r-- | js/src/js.msg | 1 |
5 files changed, 330 insertions, 35 deletions
diff --git a/js/src/frontend/NameAnalysisTypes.h b/js/src/frontend/NameAnalysisTypes.h index d39e177fb..b94ca6cc0 100644 --- a/js/src/frontend/NameAnalysisTypes.h +++ b/js/src/frontend/NameAnalysisTypes.h @@ -124,6 +124,7 @@ DeclarationKindIsLexical(DeclarationKind kind) // Used in Parser to track declared names. class DeclaredNameInfo { + uint32_t pos_; DeclarationKind kind_; // If the declared name is a binding, whether the binding is closed @@ -132,8 +133,9 @@ class DeclaredNameInfo bool closedOver_; public: - explicit DeclaredNameInfo(DeclarationKind kind) - : kind_(kind), + explicit DeclaredNameInfo(DeclarationKind kind, uint32_t pos) + : pos_(pos), + kind_(kind), closedOver_(false) { } @@ -144,6 +146,12 @@ class DeclaredNameInfo return kind_; } + static const uint32_t npos = uint32_t(-1); + + uint32_t pos() const { + return pos_; + } + void alterKind(DeclarationKind kind) { kind_ = kind; } diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index ffece5721..c96e8c065 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -19,6 +19,8 @@ #include "frontend/Parser.h" +#include "mozilla/Sprintf.h" + #include <new> #include "jsapi.h" @@ -200,11 +202,12 @@ ParseContext::Scope::addCatchParameters(ParseContext* pc, Scope& catchParamScope for (DeclaredNameMap::Range r = catchParamScope.declared_->all(); !r.empty(); r.popFront()) { DeclarationKind kind = r.front().value()->kind(); + uint32_t pos = r.front().value()->pos(); MOZ_ASSERT(DeclarationKindIsCatchParameter(kind)); JSAtom* name = r.front().key(); AddDeclaredNamePtr p = lookupDeclaredNameForAdd(name); MOZ_ASSERT(!p); - if (!addDeclaredName(pc, p, name, kind)) + if (!addDeclaredName(pc, p, name, kind, pos)) return false; } @@ -343,7 +346,8 @@ ParseContext::init() namedLambdaScope_->lookupDeclaredNameForAdd(fun->explicitName()); MOZ_ASSERT(!p); if (!namedLambdaScope_->addDeclaredName(this, p, fun->explicitName(), - DeclarationKind::Const)) + DeclarationKind::Const, + DeclaredNameInfo::npos)) { return false; } @@ -1001,13 +1005,42 @@ Parser<ParseHandler>::hasValidSimpleStrictParameterNames() template <typename ParseHandler> void -Parser<ParseHandler>::reportRedeclaration(HandlePropertyName name, DeclarationKind kind, - TokenPos pos) +Parser<ParseHandler>::reportRedeclaration(HandlePropertyName name, DeclarationKind prevKind, + TokenPos pos, uint32_t prevPos) { JSAutoByteString bytes; if (!AtomToPrintableString(context, name, &bytes)) return; - errorAt(pos.begin, JSMSG_REDECLARED_VAR, DeclarationKindString(kind), bytes.ptr()); + + if (prevPos == DeclaredNameInfo::npos) { + errorAt(pos.begin, JSMSG_REDECLARED_VAR, DeclarationKindString(prevKind), bytes.ptr()); + return; + } + + auto notes = MakeUnique<JSErrorNotes>(); + if (!notes) + return; + + uint32_t line, column; + tokenStream.srcCoords.lineNumAndColumnIndex(prevPos, &line, &column); + + const size_t MaxWidth = sizeof("4294967295"); + char columnNumber[MaxWidth]; + SprintfLiteral(columnNumber, "%" PRIu32, column); + char lineNumber[MaxWidth]; + SprintfLiteral(lineNumber, "%" PRIu32, line); + + if (!notes->addNoteLatin1(pc->sc()->context, + getFilename(), line, column, + GetErrorMessage, nullptr, + JSMSG_REDECLARED_PREV, + lineNumber, columnNumber)) + { + return; + } + + errorWithNotesAt(Move(notes), pos.begin, JSMSG_REDECLARED_VAR, + DeclarationKindString(prevKind), bytes.ptr()); } // notePositionalFormalParameter is called for both the arguments of a regular @@ -1022,6 +1055,7 @@ Parser<ParseHandler>::reportRedeclaration(HandlePropertyName name, DeclarationKi template <typename ParseHandler> bool Parser<ParseHandler>::notePositionalFormalParameter(Node fn, HandlePropertyName name, + uint32_t beginPos, bool disallowDuplicateParams, bool* duplicatedParam) { @@ -1046,7 +1080,7 @@ Parser<ParseHandler>::notePositionalFormalParameter(Node fn, HandlePropertyName *duplicatedParam = true; } else { DeclarationKind kind = DeclarationKind::PositionalFormalParameter; - if (!pc->functionScope().addDeclaredName(pc, p, name, kind)) + if (!pc->functionScope().addDeclaredName(pc, p, name, kind, beginPos)) return false; } @@ -1150,8 +1184,8 @@ DeclarationKindIsParameter(DeclarationKind kind) template <typename ParseHandler> bool -Parser<ParseHandler>::tryDeclareVar(HandlePropertyName name, DeclarationKind kind, - Maybe<DeclarationKind>* redeclaredKind) +Parser<ParseHandler>::tryDeclareVar(HandlePropertyName name, DeclarationKind kind, uint32_t beginPos, + Maybe<DeclarationKind>* redeclaredKind, uint32_t* prevPos) { MOZ_ASSERT(DeclarationKindIsVar(kind)); @@ -1224,23 +1258,29 @@ Parser<ParseHandler>::tryDeclareVar(HandlePropertyName name, DeclarationKind kin if (!annexB35Allowance && !annexB33Allowance) { *redeclaredKind = Some(declaredKind); + *prevPos = p->value()->pos(); return true; } } else if (kind == DeclarationKind::VarForAnnexBLexicalFunction) { MOZ_ASSERT(DeclarationKindIsParameter(declaredKind)); // Annex B.3.3.1 disallows redeclaring parameter names. + // We don't need to set *prevPos here since this case is not + // an error. *redeclaredKind = Some(declaredKind); return true; } } else { - if (!scope->addDeclaredName(pc, p, name, kind)) + if (!scope->addDeclaredName(pc, p, name, kind, beginPos)) return false; } } - if (!pc->sc()->strict() && pc->sc()->isEvalContext()) + if (!pc->sc()->strict() && pc->sc()->isEvalContext()) { *redeclaredKind = isVarRedeclaredInEval(name, kind); + // We don't have position information at runtime. + *prevPos = DeclaredNameInfo::npos; + } return true; } @@ -1248,11 +1288,15 @@ Parser<ParseHandler>::tryDeclareVar(HandlePropertyName name, DeclarationKind kin template <typename ParseHandler> bool Parser<ParseHandler>::tryDeclareVarForAnnexBLexicalFunction(HandlePropertyName name, - bool* tryAnnexB) + uint32_t beginPos, bool* tryAnnexB) { Maybe<DeclarationKind> redeclaredKind; - if (!tryDeclareVar(name, DeclarationKind::VarForAnnexBLexicalFunction, &redeclaredKind)) + uint32_t unused; + if (!tryDeclareVar(name, DeclarationKind::VarForAnnexBLexicalFunction, beginPos, + &redeclaredKind, &unused)) + { return false; + } if (!redeclaredKind && pc->isFunctionBox()) { ParseContext::Scope& funScope = pc->functionScope(); @@ -1324,11 +1368,12 @@ Parser<ParseHandler>::noteDeclaredName(HandlePropertyName name, DeclarationKind case DeclarationKind::BodyLevelFunction: case DeclarationKind::ForOfVar: { Maybe<DeclarationKind> redeclaredKind; - if (!tryDeclareVar(name, kind, &redeclaredKind)) + uint32_t prevPos; + if (!tryDeclareVar(name, kind, pos.begin, &redeclaredKind, &prevPos)) return false; if (redeclaredKind) { - reportRedeclaration(name, *redeclaredKind, pos); + reportRedeclaration(name, *redeclaredKind, pos, prevPos); return false; } @@ -1345,7 +1390,7 @@ Parser<ParseHandler>::noteDeclaredName(HandlePropertyName name, DeclarationKind return false; } - if (!pc->functionScope().addDeclaredName(pc, p, name, kind)) + if (!pc->functionScope().addDeclaredName(pc, p, name, kind, pos.begin)) return false; break; @@ -1368,7 +1413,7 @@ Parser<ParseHandler>::noteDeclaredName(HandlePropertyName name, DeclarationKind (p->value()->kind() != DeclarationKind::LexicalFunction && p->value()->kind() != DeclarationKind::VarForAnnexBLexicalFunction)) { - reportRedeclaration(name, p->value()->kind(), pos); + reportRedeclaration(name, p->value()->kind(), pos, p->value()->pos()); return false; } @@ -1376,7 +1421,7 @@ Parser<ParseHandler>::noteDeclaredName(HandlePropertyName name, DeclarationKind // declaration that shadows the VarForAnnexBLexicalFunction. p->value()->alterKind(kind); } else { - if (!scope->addDeclaredName(pc, p, name, kind)) + if (!scope->addDeclaredName(pc, p, name, kind, pos.begin)) return false; } @@ -1416,7 +1461,7 @@ Parser<ParseHandler>::noteDeclaredName(HandlePropertyName name, DeclarationKind if (pc->isFunctionExtraBodyVarScopeInnermost()) { DeclaredNamePtr p = pc->functionScope().lookupDeclaredName(name); if (p && DeclarationKindIsParameter(p->value()->kind())) { - reportRedeclaration(name, p->value()->kind(), pos); + reportRedeclaration(name, p->value()->kind(), pos, p->value()->pos()); return false; } } @@ -1433,12 +1478,12 @@ Parser<ParseHandler>::noteDeclaredName(HandlePropertyName name, DeclarationKind p = scope->lookupDeclaredNameForAdd(name); MOZ_ASSERT(!p); } else { - reportRedeclaration(name, p->value()->kind(), pos); + reportRedeclaration(name, p->value()->kind(), pos, p->value()->pos()); return false; } } - if (!p && !scope->addDeclaredName(pc, p, name, kind)) + if (!p && !scope->addDeclaredName(pc, p, name, kind, pos.begin)) return false; break; @@ -2188,8 +2233,11 @@ Parser<ParseHandler>::declareFunctionThis() ParseContext::Scope& funScope = pc->functionScope(); AddDeclaredNamePtr p = funScope.lookupDeclaredNameForAdd(dotThis); MOZ_ASSERT(!p); - if (!funScope.addDeclaredName(pc, p, dotThis, DeclarationKind::Var)) + if (!funScope.addDeclaredName(pc, p, dotThis, DeclarationKind::Var, + DeclaredNameInfo::npos)) + { return false; + } funbox->setHasThisBinding(); } @@ -2231,8 +2279,11 @@ Parser<ParseHandler>::declareDotGeneratorName() ParseContext::Scope& funScope = pc->functionScope(); HandlePropertyName dotGenerator = context->names().dotGenerator; AddDeclaredNamePtr p = funScope.lookupDeclaredNameForAdd(dotGenerator); - if (!p && !funScope.addDeclaredName(pc, p, dotGenerator, DeclarationKind::Var)) + if (!p && !funScope.addDeclaredName(pc, p, dotGenerator, DeclarationKind::Var, + DeclaredNameInfo::npos)) + { return false; + } return true; } @@ -2481,8 +2532,11 @@ Parser<ParseHandler>::declareFunctionArgumentsObject() if (tryDeclareArguments) { AddDeclaredNamePtr p = funScope.lookupDeclaredNameForAdd(argumentsName); if (!p) { - if (!funScope.addDeclaredName(pc, p, argumentsName, DeclarationKind::Var)) + if (!funScope.addDeclaredName(pc, p, argumentsName, DeclarationKind::Var, + DeclaredNameInfo::npos)) + { return false; + } funbox->declaredArguments = true; funbox->usesArguments = true; } else if (hasExtraBodyVarScope) { @@ -2964,8 +3018,8 @@ Parser<ParseHandler>::functionArguments(YieldHandling yieldHandling, FunctionSyn if (!name) return false; - if (!notePositionalFormalParameter(funcpn, name, disallowDuplicateParams, - &duplicatedParam)) + if (!notePositionalFormalParameter(funcpn, name, pos().begin, + disallowDuplicateParams, &duplicatedParam)) { return false; } @@ -3689,7 +3743,7 @@ Parser<ParseHandler>::functionStmt(uint32_t preludeStart, YieldHandling yieldHan // early error, do so. This 'var' binding would be assigned // the function object when its declaration is reached, not at // the start of the block. - if (!tryDeclareVarForAnnexBLexicalFunction(name, &tryAnnexB)) + if (!tryDeclareVarForAnnexBLexicalFunction(name, pos().begin, &tryAnnexB)) return null(); } diff --git a/js/src/frontend/Parser.h b/js/src/frontend/Parser.h index 824dddb58..6b5ade425 100644 --- a/js/src/frontend/Parser.h +++ b/js/src/frontend/Parser.h @@ -146,9 +146,9 @@ class ParseContext : public Nestable<ParseContext> } MOZ_MUST_USE bool addDeclaredName(ParseContext* pc, AddDeclaredNamePtr& p, JSAtom* name, - DeclarationKind kind) + DeclarationKind kind, uint32_t pos) { - return maybeReportOOM(pc, declared_->add(p, name, DeclaredNameInfo(kind))); + return maybeReportOOM(pc, declared_->add(p, name, DeclaredNameInfo(kind, pos))); } // Remove all VarForAnnexBLexicalFunction declarations of a certain @@ -1437,15 +1437,17 @@ class Parser final : public ParserBase, private JS::AutoGCRooter bool hasValidSimpleStrictParameterNames(); - void reportRedeclaration(HandlePropertyName name, DeclarationKind kind, TokenPos pos); - bool notePositionalFormalParameter(Node fn, HandlePropertyName name, + void reportRedeclaration(HandlePropertyName name, DeclarationKind prevKind, TokenPos pos, + uint32_t prevPos); + bool notePositionalFormalParameter(Node fn, HandlePropertyName name, uint32_t beginPos, bool disallowDuplicateParams, bool* duplicatedParam); bool noteDestructuredPositionalFormalParameter(Node fn, Node destruct); mozilla::Maybe<DeclarationKind> isVarRedeclaredInEval(HandlePropertyName name, DeclarationKind kind); - bool tryDeclareVar(HandlePropertyName name, DeclarationKind kind, - mozilla::Maybe<DeclarationKind>* redeclaredKind); - bool tryDeclareVarForAnnexBLexicalFunction(HandlePropertyName name, bool* tryAnnexB); + bool tryDeclareVar(HandlePropertyName name, DeclarationKind kind, uint32_t beginPos, + mozilla::Maybe<DeclarationKind>* redeclaredKind, uint32_t* prevPos); + bool tryDeclareVarForAnnexBLexicalFunction(HandlePropertyName name, uint32_t beginPos, + bool* tryAnnexB); bool checkLexicalDeclarationDirectlyWithinBlock(ParseContext::Statement& stmt, DeclarationKind kind, TokenPos pos); bool noteDeclaredName(HandlePropertyName name, DeclarationKind kind, TokenPos pos); diff --git a/js/src/jit-test/tests/parser/redeclaration.js b/js/src/jit-test/tests/parser/redeclaration.js new file mode 100644 index 000000000..f719021ac --- /dev/null +++ b/js/src/jit-test/tests/parser/redeclaration.js @@ -0,0 +1,230 @@ +// Error message for redeclaration should show the position where the variable +// was declared. + +const npos = -1; + +function test_one(fun, filename, name, + [prevLineNumber, prevColumnNumber], + [lineNumber, columnNumber]) { + let caught = false; + try { + fun(); + } catch (e) { + assertEq(e.message.includes("redeclaration"), true); + assertEq(e.lineNumber, lineNumber); + assertEq(e.columnNumber, columnNumber); + let notes = getErrorNotes(e); + if (prevLineNumber == npos) { + assertEq(notes.length, 0); + } else { + assertEq(notes.length, 1); + let note = notes[0]; + assertEq(note.message, + `Previously declared at line ${prevLineNumber}, column ${prevColumnNumber}`); + assertEq(note.lineNumber, prevLineNumber); + assertEq(note.columnNumber, prevColumnNumber); + if (filename) + assertEq(note.fileName, filename); + } + caught = true; + } + assertEq(caught, true); +} + +function test_parse(source, ...args) { + test_one(() => { + Reflect.parse(source, { source: "foo.js" }); + }, "foo.js", ...args); +} + +function test_eval(source, ...args) { + test_one(() => { + eval(source); + }, undefined, ...args); +} + +function test(...args) { + test_parse(...args); + test_eval(...args); +} + +// let + +test(` +let a, a; +`, "a", [2, 4], [2, 7]); + +test(` +let a; +let a; +`, "a", [2, 4], [3, 4]); + +test(` +let a; +const a = 1; +`, "a", [2, 4], [3, 6]); + +test(` +let a; +var a; +`, "a", [2, 4], [3, 4]); + +test(` +let a; +function a() { +} +`, "a", [2, 4], [3, 9]); + +test(` +{ + let a; + function a() { + } +} +`, "a", [3, 6], [4, 11]); + +// const + +test(` +const a = 1, a = 2; +`, "a", [2, 6], [2, 13]); + +test(` +const a = 1; +const a = 2; +`, "a", [2, 6], [3, 6]); + +test(` +const a = 1; +let a; +`, "a", [2, 6], [3, 4]); + +test(` +const a = 1; +var a; +`, "a", [2, 6], [3, 4]); + +test(` +const a = 1; +function a() { +} +`, "a", [2, 6], [3, 9]); + +test(` +{ + const a = 1; + function a() { + } +} +`, "a", [3, 8], [4, 11]); + +// var + +test(` +var a; +let a; +`, "a", [2, 4], [3, 4]); + +test(` +var a; +const a = 1; +`, "a", [2, 4], [3, 6]); + +// function + +test(` +function a() {}; +let a; +`, "a", [2, 9], [3, 4]); + +test(` +function a() {}; +const a = 1; +`, "a", [2, 9], [3, 6]); + +// Annex B lexical function + +test(` +{ + function a() {}; + let a; +} +`, "a", [3, 11], [4, 6]); + +test(` +{ + function a() {}; + const a = 1; +} +`, "a", [3, 11], [4, 8]); + +// catch parameter + +test(` +try { +} catch (a) { + let a; +} +`, "a", [3, 9], [4, 6]); + +test(` +try { +} catch (a) { + const a = 1; +} +`, "a", [3, 9], [4, 8]); + +test(` +try { +} catch (a) { + function a() { + } +} +`, "a", [3, 9], [4, 11]); + +// parameter + +test(` +function f(a) { + let a; +} +`, "a", [2, 11], [3, 6]); + +test(` +function f(a) { + const a = 1; +} +`, "a", [2, 11], [3, 8]); + +test(` +function f([a]) { + let a; +} +`, "a", [2, 12], [3, 6]); + +test(` +function f({a}) { + let a; +} +`, "a", [2, 12], [3, 6]); + +test(` +function f(...a) { + let a; +} +`, "a", [2, 14], [3, 6]); + +test(` +function f(a=1) { + let a; +} +`, "a", [2, 11], [3, 6]); + +// eval +// We don't have position information at runtime. +// No note should be shown. + +test_eval(` +let a; +eval("var a"); +`, "a", [npos, npos], [1, 4]); diff --git a/js/src/js.msg b/js/src/js.msg index ce039cbc8..569928be8 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -63,6 +63,7 @@ MSG_DEF(JSMSG_SPREAD_TOO_LARGE, 0, JSEXN_RANGEERR, "array too large due t MSG_DEF(JSMSG_BAD_WEAKMAP_KEY, 0, JSEXN_TYPEERR, "cannot use the given object as a weak map key") MSG_DEF(JSMSG_BAD_GETTER_OR_SETTER, 1, JSEXN_TYPEERR, "invalid {0} usage") MSG_DEF(JSMSG_BAD_ARRAY_LENGTH, 0, JSEXN_RANGEERR, "invalid array length") +MSG_DEF(JSMSG_REDECLARED_PREV, 2, JSEXN_NOTE, "Previously declared at line {0}, column {1}") MSG_DEF(JSMSG_REDECLARED_VAR, 2, JSEXN_SYNTAXERR, "redeclaration of {0} {1}") MSG_DEF(JSMSG_UNDECLARED_VAR, 1, JSEXN_REFERENCEERR, "assignment to undeclared variable {0}") MSG_DEF(JSMSG_GETTER_ONLY, 1, JSEXN_TYPEERR, "setting getter-only property {0}") |