From e5ccb0b9ff574850ad5079546da36f448ee37221 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Wed, 18 Nov 2009 14:36:55 -0800 Subject: [PATCH] Back out ad37dab7ea17: code to check the current frame's script's strictness flag fails the static checks. Using js_GetTopStackFrame introduces a perf regression. --- js/src/jsapi.h | 11 -------- js/src/jscntxt.cpp | 50 ++++++++++------------------------ js/src/jscntxt.h | 2 +- js/src/jsscan.cpp | 67 ++++++---------------------------------------- js/src/jsscan.h | 23 +--------------- 5 files changed, 24 insertions(+), 129 deletions(-) diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 7e15b36b4a41..ab0524121016 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2684,15 +2684,6 @@ struct JSErrorReport { #define JSREPORT_EXCEPTION 0x2 /* exception was thrown */ #define JSREPORT_STRICT 0x4 /* error or warning due to strict option */ -/* - * This condition is an error in strict mode code, a warning if - * JS_HAS_STRICT_OPTION(cx), and otherwise should not be reported at - * all. We check the strictness of the context's top frame's script; - * where that isn't appropriate, the caller should do the right checks - * itself instead of using this flag. - */ -#define JSREPORT_STRICT_MODE_ERROR 0x8 - /* * If JSREPORT_EXCEPTION is set, then a JavaScript-catchable exception * has been thrown for this runtime error, and the host should ignore it. @@ -2703,8 +2694,6 @@ struct JSErrorReport { #define JSREPORT_IS_WARNING(flags) (((flags) & JSREPORT_WARNING) != 0) #define JSREPORT_IS_EXCEPTION(flags) (((flags) & JSREPORT_EXCEPTION) != 0) #define JSREPORT_IS_STRICT(flags) (((flags) & JSREPORT_STRICT) != 0) -#define JSREPORT_IS_STRICT_MODE_ERROR(flags) (((flags) & \ - JSREPORT_STRICT_MODE_ERROR) != 0) extern JS_PUBLIC_API(JSErrorReporter) JS_SetErrorReporter(JSContext *cx, JSErrorReporter er); diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 3afc32dd3a25..9aa951a8dbf6 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -1401,37 +1401,6 @@ js_ReportAllocationOverflow(JSContext *cx) JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_ALLOC_OVERFLOW); } -/* - * Given flags and the state of cx, decide whether we should report an - * error, a warning, or just continue execution normally. Return - * true if we should continue normally, without reporting anything; - * otherwise, adjust *flags as appropriate and return false. - */ -static bool -checkReportFlags(JSContext *cx, uintN *flags) -{ - if (JSREPORT_IS_STRICT_MODE_ERROR(*flags)) { - /* Error in strict code; warning with strict option; okay otherwise. */ - JS_ASSERT(JS_IsRunning(cx)); - if (cx->fp->script->strictModeCode) - *flags &= ~JSREPORT_WARNING; - else if (JS_HAS_STRICT_OPTION(cx)) - *flags |= JSREPORT_WARNING; - else - return true; - } else if (JSREPORT_IS_STRICT(*flags)) { - /* Warning/error only when JSOPTION_STRICT is set. */ - if (!JS_HAS_STRICT_OPTION(cx)) - return true; - } - - /* Warnings become errors when JSOPTION_WERROR is set. */ - if (JSREPORT_IS_WARNING(*flags) && JS_HAS_WERROR_OPTION(cx)) - *flags &= ~JSREPORT_WARNING; - - return false; -} - JSBool js_ReportErrorVA(JSContext *cx, uintN flags, const char *format, va_list ap) { @@ -1441,7 +1410,7 @@ js_ReportErrorVA(JSContext *cx, uintN flags, const char *format, va_list ap) JSErrorReport report; JSBool warning; - if (checkReportFlags(cx, &flags)) + if ((flags & JSREPORT_STRICT) && !JS_HAS_STRICT_OPTION(cx)) return JS_TRUE; message = JS_vsmprintf(format, ap); @@ -1456,6 +1425,10 @@ js_ReportErrorVA(JSContext *cx, uintN flags, const char *format, va_list ap) PopulateReportBlame(cx, &report); warning = JSREPORT_IS_WARNING(report.flags); + if (warning && JS_HAS_WERROR_OPTION(cx)) { + report.flags &= ~JSREPORT_WARNING; + warning = JS_FALSE; + } ReportError(cx, message, &report); js_free(message); @@ -1478,12 +1451,18 @@ JSBool js_ExpandErrorArguments(JSContext *cx, JSErrorCallback callback, void *userRef, const uintN errorNumber, char **messagep, JSErrorReport *reportp, - bool charArgs, va_list ap) + JSBool *warningp, JSBool charArgs, va_list ap) { const JSErrorFormatString *efs; int i; int argCount; + *warningp = JSREPORT_IS_WARNING(reportp->flags); + if (*warningp && JS_HAS_WERROR_OPTION(cx)) { + reportp->flags &= ~JSREPORT_WARNING; + *warningp = JS_FALSE; + } + *messagep = NULL; /* Most calls supply js_GetErrorMessage; if this is so, assume NULL. */ @@ -1637,9 +1616,8 @@ js_ReportErrorNumberVA(JSContext *cx, uintN flags, JSErrorCallback callback, char *message; JSBool warning; - if (checkReportFlags(cx, &flags)) + if ((flags & JSREPORT_STRICT) && !JS_HAS_STRICT_OPTION(cx)) return JS_TRUE; - warning = JSREPORT_IS_WARNING(flags); memset(&report, 0, sizeof (struct JSErrorReport)); report.flags = flags; @@ -1647,7 +1625,7 @@ js_ReportErrorNumberVA(JSContext *cx, uintN flags, JSErrorCallback callback, PopulateReportBlame(cx, &report); if (!js_ExpandErrorArguments(cx, callback, userRef, errorNumber, - &message, &report, charArgs, ap)) { + &message, &report, &warning, charArgs, ap)) { return JS_FALSE; } diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 390520bab01d..760722cbc2aa 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -1777,7 +1777,7 @@ extern JSBool js_ExpandErrorArguments(JSContext *cx, JSErrorCallback callback, void *userRef, const uintN errorNumber, char **message, JSErrorReport *reportp, - bool charArgs, va_list ap); + JSBool *warningp, JSBool charArgs, va_list ap); #endif extern void diff --git a/js/src/jsscan.cpp b/js/src/jsscan.cpp index 77b55192b0bd..00b79d283f47 100644 --- a/js/src/jsscan.cpp +++ b/js/src/jsscan.cpp @@ -494,15 +494,16 @@ MatchChar(JSTokenStream *ts, int32 expect) return JS_FALSE; } -static bool -ReportCompileErrorNumberVA(JSContext *cx, JSTokenStream *ts, JSParseNode *pn, - uintN flags, uintN errorNumber, va_list ap) +JSBool +js_ReportCompileErrorNumber(JSContext *cx, JSTokenStream *ts, JSParseNode *pn, + uintN flags, uintN errorNumber, ...) { JSErrorReport report; char *message; size_t linelength; jschar *linechars; char *linebytes; + va_list ap; JSBool warning, ok; JSTokenPos *tp; uintN index, i; @@ -510,15 +511,9 @@ ReportCompileErrorNumberVA(JSContext *cx, JSTokenStream *ts, JSParseNode *pn, JS_ASSERT(ts->linebuf.limit < ts->linebuf.base + JS_LINE_LIMIT); - if (JSREPORT_IS_STRICT(flags) && !JS_HAS_STRICT_OPTION(cx)) + if ((flags & JSREPORT_STRICT) && !JS_HAS_STRICT_OPTION(cx)) return JS_TRUE; - warning = JSREPORT_IS_WARNING(flags); - if (warning && JS_HAS_WERROR_OPTION(cx)) { - flags &= ~JSREPORT_WARNING; - warning = false; - } - memset(&report, 0, sizeof report); report.flags = flags; report.errorNumber = errorNumber; @@ -527,9 +522,11 @@ ReportCompileErrorNumberVA(JSContext *cx, JSTokenStream *ts, JSParseNode *pn, linebytes = NULL; MUST_FLOW_THROUGH("out"); + va_start(ap, errorNumber); ok = js_ExpandErrorArguments(cx, js_GetErrorMessage, NULL, - errorNumber, &message, &report, + errorNumber, &message, &report, &warning, !(flags & JSREPORT_UC), ap); + va_end(ap); if (!ok) { warning = JS_FALSE; goto out; @@ -662,54 +659,6 @@ ReportCompileErrorNumberVA(JSContext *cx, JSTokenStream *ts, JSParseNode *pn, return warning; } -bool -js_ReportStrictModeError(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc, - JSParseNode *pn, uintN errorNumber, ...) -{ - bool result; - va_list ap; - uintN flags; - - JS_ASSERT(ts || tc); - - /* In strict mode code, this is an error, not just a warning. */ - if ((tc && tc->flags & TCF_STRICT_MODE_CODE) || - (ts && ts->flags & TSF_STRICT_MODE_CODE)) { - flags = JSREPORT_ERROR; - } else if (JS_HAS_STRICT_OPTION(cx)) { - flags = JSREPORT_WARNING; - } else { - return true; - } - - va_start(ap, errorNumber); - result = ReportCompileErrorNumberVA(cx, ts, pn, flags, errorNumber, ap); - va_end(ap); - - return result; -} - -bool -js_ReportCompileErrorNumber(JSContext *cx, JSTokenStream *ts, JSParseNode *pn, - uintN flags, uintN errorNumber, ...) -{ - bool result; - va_list ap; - - /* - * We don't accept a JSTreeContext argument, so we can't implement - * JSREPORT_STRICT_MODE_ERROR here. Use js_ReportStrictModeError instead, - * or do the checks in the caller and pass plain old JSREPORT_ERROR. - */ - JS_ASSERT(!(flags & JSREPORT_STRICT_MODE_ERROR)); - - va_start(ap, errorNumber); - result = ReportCompileErrorNumberVA(cx, ts, pn, flags, errorNumber, ap); - va_end(ap); - - return result; -} - #if JS_HAS_XML_SUPPORT static JSBool diff --git a/js/src/jsscan.h b/js/src/jsscan.h index 511af74fd376..4de2367e0e9b 100644 --- a/js/src/jsscan.h +++ b/js/src/jsscan.h @@ -384,31 +384,10 @@ js_IsIdentifier(JSString *str); * for an error. When pn is not null, use it to report error's location. * Otherwise use ts, which must not be null. */ -bool +JSBool js_ReportCompileErrorNumber(JSContext *cx, JSTokenStream *ts, JSParseNode *pn, uintN flags, uintN errorNumber, ...); -/* - * Report a condition that should elicit a warning with JSOPTION_STRICT, - * or an error if ts or tc is handling strict mode code. This function - * defers to js_ReportCompileErrorNumber to do the real work. Either tc - * or ts may be NULL, if there is no tree context or token stream state - * whose strictness should affect the report. - * - * One could have js_ReportCompileErrorNumber recognize the - * JSREPORT_STRICT_MODE_ERROR flag instead of having a separate function - * like this one. However, the strict mode code flag we need to test is - * in the JSTreeContext structure for that code; we would have to change - * the ~120 js_ReportCompileErrorNumber calls to pass the additional - * argument, even though many of those sites would never use it. Using - * ts's TSF_STRICT_MODE_CODE flag instead of tc's would be brittle: at some - * points ts's flags don't correspond to those of the tc relevant to the - * error. - */ -bool -js_ReportStrictModeError(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc, - JSParseNode *pn, uintN errorNumber, ...); - /* * Steal one JSREPORT_* bit (see jsapi.h) to tell that arguments to the error * message have const jschar* type, not const char*.