Bug 1622562 - Remove flags from JSErrorReport. r=jandem
authorTom Schuster <evilpies@gmail.com>
Mon, 23 Mar 2020 07:08:48 +0000
changeset 520204 d70d72a60f12791b7f5c5aef535c7a09be089d12
parent 520203 2c061e58efbfa989d7379a9e1456dca3d8908014
child 520205 a5228fc5e5b371bc46963553e6365c27089f3932
push id37245
push useropoprus@mozilla.com
push dateTue, 24 Mar 2020 21:46:41 +0000
treeherdermozilla-central@dbabf2e388fa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1622562
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1622562 - Remove flags from JSErrorReport. r=jandem Differential Revision: https://phabricator.services.mozilla.com/D67143
js/public/ErrorReport.h
js/src/jsexn.cpp
js/src/vm/ErrorReporting.cpp
js/src/vm/JSContext.cpp
js/src/vm/JSContext.h
--- a/js/public/ErrorReport.h
+++ b/js/public/ErrorReport.h
@@ -198,26 +198,16 @@ class JSErrorNotes {
     }
     reference operator*() { return *note_; }
   };
 
   JS_PUBLIC_API iterator begin();
   JS_PUBLIC_API iterator end();
 };
 
-/*
- * JSErrorReport flag values.  These may be freely composed.
- *
- * To be removed.
- */
-#define JSREPORT_ERROR 0x0   /* pseudo-flag for default case */
-#define JSREPORT_WARNING 0x1 /* reported via JS::Warn* */
-
-#define JSREPORT_USER_1 0x8 /* user-defined flag */
-
 /**
  * Describes a single error or warning that occurs in the execution of script.
  */
 class JSErrorReport : public JSErrorBase {
  private:
   // Offending source line without final '\n'.
   // If ownsLinebuf_ is true, the buffer is freed in destructor.
   const char16_t* linebuf_;
@@ -227,52 +217,52 @@ class JSErrorReport : public JSErrorBase
 
   // The 0-based offset of error token in linebuf_.
   size_t tokenOffset_;
 
  public:
   // Associated notes, or nullptr if there's no note.
   js::UniquePtr<JSErrorNotes> notes;
 
-  // error/warning, etc.
-  unsigned flags;
-
   // One of the JSExnType constants.
   int16_t exnType;
 
   // See the comment in TransitiveCompileOptions.
   bool isMuted : 1;
 
+  // This error report is actually a warning.
+  bool isWarning_ : 1;
+
  private:
   bool ownsLinebuf_ : 1;
 
  public:
   JSErrorReport()
       : linebuf_(nullptr),
         linebufLength_(0),
         tokenOffset_(0),
         notes(nullptr),
-        flags(0),
         exnType(0),
         isMuted(false),
+        isWarning_(false),
         ownsLinebuf_(false) {}
 
   ~JSErrorReport() { freeLinebuf(); }
 
  public:
   const char16_t* linebuf() const { return linebuf_; }
   size_t linebufLength() const { return linebufLength_; }
   size_t tokenOffset() const { return tokenOffset_; }
   void initOwnedLinebuf(const char16_t* linebufArg, size_t linebufLengthArg,
                         size_t tokenOffsetArg) {
     initBorrowedLinebuf(linebufArg, linebufLengthArg, tokenOffsetArg);
     ownsLinebuf_ = true;
   }
   void initBorrowedLinebuf(const char16_t* linebufArg, size_t linebufLengthArg,
                            size_t tokenOffsetArg);
 
-  bool isWarning() const { return !!(flags & JSREPORT_WARNING); }
+  bool isWarning() const { return isWarning_; }
 
  private:
   void freeLinebuf();
 };
 
 #endif /* js_ErrorReport_h */
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -91,17 +91,17 @@ bool CopyExtraData(JSContext* cx, uint8_
     *cursor += linebufSize + alignment_backlog;
     copy->initBorrowedLinebuf(linebufCopy, report->linebufLength(),
                               report->tokenOffset());
   }
 
   /* Copy non-pointer members. */
   copy->isMuted = report->isMuted;
   copy->exnType = report->exnType;
-  copy->flags = report->flags;
+  copy->isWarning_ = report->isWarning_;
 
   /* Deep copy notes. */
   if (report->notes) {
     auto copiedNotes = report->notes->copy(cx);
     if (!copiedNotes) {
       return false;
     }
     copy->notes = std::move(copiedNotes);
@@ -622,17 +622,17 @@ bool ErrorReport::populateUncaughtExcept
   bool ok = populateUncaughtExceptionReportUTF8VA(cx, ap);
   va_end(ap);
   return ok;
 }
 
 bool ErrorReport::populateUncaughtExceptionReportUTF8VA(JSContext* cx,
                                                         va_list ap) {
   new (&ownedReport) JSErrorReport();
-  ownedReport.flags = JSREPORT_ERROR;
+  ownedReport.isWarning_ = false;
   ownedReport.errorNumber = JSMSG_UNCAUGHT_EXCEPTION;
   // XXXbz this assumes the stack we have right now is still
   // related to our exception object.  It would be better if we
   // could accept a passed-in stack of some sort instead.
   NonBuiltinFrameIter iter(cx, cx->realm()->principals());
   if (!iter.done()) {
     ownedReport.filename = iter.filename();
     uint32_t column;
--- a/js/src/vm/ErrorReporting.cpp
+++ b/js/src/vm/ErrorReporting.cpp
@@ -58,17 +58,17 @@ bool js::ReportCompileWarning(JSContext*
   // it later.
   CompileError tempErr;
   CompileError* err = &tempErr;
   if (cx->isHelperThreadContext() && !cx->addPendingCompileError(&err)) {
     return false;
   }
 
   err->notes = std::move(notes);
-  err->flags = JSREPORT_WARNING;
+  err->isWarning_ = true;
   err->errorNumber = errorNumber;
 
   err->filename = metadata.filename;
   err->lineno = metadata.lineNumber;
   err->column = metadata.columnNumber;
   err->isMuted = metadata.isMuted;
 
   if (UniqueTwoByteChars lineOfContext = std::move(metadata.lineOfContext)) {
@@ -97,17 +97,17 @@ static void ReportCompileErrorImpl(JSCon
   // it later.
   js::CompileError tempErr;
   js::CompileError* err = &tempErr;
   if (cx->isHelperThreadContext() && !cx->addPendingCompileError(&err)) {
     return;
   }
 
   err->notes = std::move(notes);
-  err->flags = JSREPORT_ERROR;
+  err->isWarning_ = false;
   err->errorNumber = errorNumber;
 
   err->filename = metadata.filename;
   err->lineno = metadata.lineNumber;
   err->column = metadata.columnNumber;
   err->isMuted = metadata.isMuted;
 
   if (UniqueTwoByteChars lineOfContext = std::move(metadata.lineOfContext)) {
@@ -460,18 +460,17 @@ bool js::ExpandErrorArgumentsVA(JSContex
                                     messageArgs, argumentsType, notep, ap);
 }
 
 bool js::ReportErrorNumberVA(JSContext* cx, IsWarning isWarning,
                              JSErrorCallback callback, void* userRef,
                              const unsigned errorNumber,
                              ErrorArgumentsType argumentsType, va_list ap) {
   JSErrorReport report;
-  report.flags =
-      isWarning == IsWarning::Yes ? JSREPORT_WARNING : JSREPORT_ERROR;
+  report.isWarning_ = isWarning == IsWarning::Yes;
   report.errorNumber = errorNumber;
   PopulateReportBlame(cx, &report);
 
   if (!ExpandErrorArgumentsVA(cx, callback, userRef, errorNumber, argumentsType,
                               &report, ap)) {
     return false;
   }
 
@@ -501,18 +500,17 @@ static bool ReportErrorNumberArray(JSCon
                                    const unsigned errorNumber,
                                    const CharT** args) {
   static_assert(
       (argType == ArgumentsAreUnicode && std::is_same_v<CharT, char16_t>) ||
           (argType != ArgumentsAreUnicode && std::is_same_v<CharT, char>),
       "Mismatch between character type and argument type");
 
   JSErrorReport report;
-  report.flags =
-      isWarning == IsWarning::Yes ? JSREPORT_WARNING : JSREPORT_ERROR;
+  report.isWarning_ = isWarning == IsWarning::Yes;
   report.errorNumber = errorNumber;
   PopulateReportBlame(cx, &report);
 
   if (!ExpandErrorArguments(cx, callback, userRef, errorNumber, args, argType,
                             &report)) {
     return false;
   }
 
@@ -545,18 +543,17 @@ bool js::ReportErrorVA(JSContext* cx, Is
   if (!message) {
     ReportOutOfMemory(cx);
     return false;
   }
 
   MOZ_ASSERT_IF(argumentsType == ArgumentsAreASCII,
                 JS::StringIsASCII(message.get()));
 
-  report.flags =
-      isWarning == IsWarning::Yes ? JSREPORT_WARNING : JSREPORT_ERROR;
+  report.isWarning_ = isWarning == IsWarning::Yes;
   report.errorNumber = JSMSG_USER_DEFINED_ERROR;
   if (argumentsType == ArgumentsAreASCII || argumentsType == ArgumentsAreUTF8) {
     report.initOwnedMessage(message.release());
   } else {
     MOZ_ASSERT(argumentsType == ArgumentsAreLatin1);
     Latin1Chars latin1(message.get(), strlen(message.get()));
     UTF8CharsZ utf8(JS::CharsToNewUTF8CharsZ(cx, latin1));
     if (!utf8) {
--- a/js/src/vm/JSContext.cpp
+++ b/js/src/vm/JSContext.cpp
@@ -368,17 +368,17 @@ static void PrintErrorLine(FILE* file, c
     fputc('^', file);
   }
 }
 
 static void PrintErrorLine(FILE* file, const char* prefix,
                            JSErrorNotes::Note* note) {}
 
 template <typename T>
-static bool PrintSingleError(JSContext* cx, FILE* file,
+static void PrintSingleError(JSContext* cx, FILE* file,
                              JS::ConstUTF8CharsZ toStringResult, T* report,
                              PrintErrorKind kind) {
   UniqueChars prefix;
   if (report->filename) {
     prefix = JS_smprintf("%s:", report->filename);
   }
 
   if (report->lineno) {
@@ -421,43 +421,40 @@ static bool PrintSingleError(JSContext* 
     fputs(prefix.get(), file);
   }
   fputs(message, file);
 
   PrintErrorLine(file, prefix.get(), report);
   fputc('\n', file);
 
   fflush(file);
-  return true;
 }
 
-bool js::PrintError(JSContext* cx, FILE* file,
+void js::PrintError(JSContext* cx, FILE* file,
                     JS::ConstUTF8CharsZ toStringResult, JSErrorReport* report,
                     bool reportWarnings) {
   MOZ_ASSERT(report);
 
   /* Conditionally ignore reported warnings. */
   if (report->isWarning() && !reportWarnings) {
-    return false;
+    return;
   }
 
   PrintErrorKind kind = PrintErrorKind::Error;
   if (report->isWarning()) {
     kind = PrintErrorKind::Warning;
   }
   PrintSingleError(cx, file, toStringResult, report, kind);
 
   if (report->notes) {
     for (auto&& note : *report->notes) {
       PrintSingleError(cx, file, JS::ConstUTF8CharsZ(), note.get(),
                        PrintErrorKind::Note);
     }
   }
-
-  return true;
 }
 
 void js::ReportIsNotDefined(JSContext* cx, HandleId id) {
   if (UniqueChars printable =
           IdToPrintableUTF8(cx, id, IdToPrintableBehavior::IdIsIdentifier)) {
     JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr, JSMSG_NOT_DEFINED,
                              printable.get());
   }
--- a/js/src/vm/JSContext.h
+++ b/js/src/vm/JSContext.h
@@ -1049,23 +1049,20 @@ struct MOZ_RAII AutoResolving {
 extern JSContext* NewContext(uint32_t maxBytes, JSRuntime* parentRuntime);
 
 extern void DestroyContext(JSContext* cx);
 
 /* |callee| requires a usage string provided by JS_DefineFunctionsWithHelp. */
 extern void ReportUsageErrorASCII(JSContext* cx, HandleObject callee,
                                   const char* msg);
 
-/*
- * Prints a full report and returns true if the given report is non-nullptr
- * and the report doesn't have the JSREPORT_WARNING flag set or reportWarnings
- * is true.
- * Returns false otherwise.
- */
-extern bool PrintError(JSContext* cx, FILE* file,
+// Writes a full report to a file descriptor.
+// Does nothing for JSErrorReport which are warnings, unless
+// reportWarnings is set.
+extern void PrintError(JSContext* cx, FILE* file,
                        JS::ConstUTF8CharsZ toStringResult,
                        JSErrorReport* report, bool reportWarnings);
 
 extern void ReportIsNotDefined(JSContext* cx, HandlePropertyName name);
 
 extern void ReportIsNotDefined(JSContext* cx, HandleId id);
 
 /*