Skip to content

Commit

Permalink
Bug 1847469 - Part 7: Use column number types in FrameIter and SavedF…
Browse files Browse the repository at this point in the history
…rame internal. r=iain

Differential Revision: https://phabricator.services.mozilla.com/D185745
  • Loading branch information
arai-a committed Aug 16, 2023
1 parent 41cb19f commit 7bb2ff0
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 83 deletions.
10 changes: 6 additions & 4 deletions js/src/frontend/TokenStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
#include "frontend/ParserAtom.h"
#include "frontend/ReservedWords.h"
#include "js/CharacterEncoding.h" // JS::ConstUTF8CharsZ
#include "js/ColumnNumber.h" // JS::LimitedColumnNumberZeroOrigin, JS::ColumnNumberZeroOrigin
#include "js/ErrorReport.h" // JSErrorBase
#include "js/ColumnNumber.h" // JS::LimitedColumnNumberZeroOrigin, JS::ColumnNumberZeroOrigin, JS::TaggedColumnNumberZeroOrigin
#include "js/ErrorReport.h" // JSErrorBase
#include "js/friend/ErrorMessages.h" // js::GetErrorMessage, JSMSG_*
#include "js/Printf.h" // JS_smprintf
#include "js/RegExpFlags.h" // JS::RegExpFlags
Expand Down Expand Up @@ -1477,9 +1477,11 @@ bool TokenStreamAnyChars::fillExceptingContext(ErrorMetadata* err,
maybeCx->realm()->principals());
if (!iter.done() && iter.filename()) {
err->filename = JS::ConstUTF8CharsZ(iter.filename());
uint32_t columnNumber;
JS::TaggedColumnNumberZeroOrigin columnNumber;
err->lineNumber = iter.computeLine(&columnNumber);
err->columnNumber = JS::ColumnNumberZeroOrigin(columnNumber);
// NOTE: Wasm frame cannot appear here.
err->columnNumber =
JS::ColumnNumberZeroOrigin(columnNumber.toLimitedColumnNumber());
return false;
}
}
Expand Down
11 changes: 9 additions & 2 deletions js/src/jsapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "jit/JitSpewer.h"
#include "js/CallAndConstruct.h" // JS::IsCallable
#include "js/CharacterEncoding.h"
#include "js/ColumnNumber.h" // JS::TaggedColumnNumberZeroOrigin
#include "js/CompileOptions.h"
#include "js/ContextOptions.h" // JS::ContextOptions{,Ref}
#include "js/Conversions.h"
Expand Down Expand Up @@ -4563,9 +4564,15 @@ JS_PUBLIC_API bool DescribeScriptedCaller(JSContext* cx, AutoFilename* filename,
}

if (lineno) {
*lineno = i.computeLine(column);
JS::TaggedColumnNumberZeroOrigin columnNumber;
*lineno = i.computeLine(&columnNumber);
if (column) {
*column = columnNumber.zeroOriginValue();
}
} else if (column) {
i.computeLine(column);
JS::TaggedColumnNumberZeroOrigin columnNumber;
i.computeLine(&columnNumber);
*column = columnNumber.zeroOriginValue();
}

return true;
Expand Down
8 changes: 4 additions & 4 deletions js/src/jsexn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "frontend/FrontendContext.h" // AutoReportFrontendContext
#include "js/CharacterEncoding.h" // JS::UTF8Chars, JS::ConstUTF8CharsZ
#include "js/Class.h"
#include "js/ColumnNumber.h" // JS::TaggedColumnNumberZeroOrigin
#include "js/Conversions.h"
#include "js/ErrorReport.h" // JS::PrintError
#include "js/Exception.h" // JS::ExceptionStack
Expand Down Expand Up @@ -674,20 +675,19 @@ bool JS::ErrorReportBuilder::populateUncaughtExceptionReportUTF8VA(
ownedReport.filename = JS::ConstUTF8CharsZ(filename.get());
ownedReport.sourceId = frame->getSourceId();
ownedReport.lineno = frame->getLine();
// Follow FixupMaybeWASMColumnForDisplay and set column to 1 for WASM.
ownedReport.column = frame->isWasm() ? 1 : frame->getColumn();
ownedReport.column = frame->getColumn().oneOriginValue();
ownedReport.isMuted = frame->getMutedErrors();
} else {
// XXXbz this assumes the stack we have right now is still
// related to our exception object.
NonBuiltinFrameIter iter(cx, cx->realm()->principals());
if (!iter.done()) {
ownedReport.filename = JS::ConstUTF8CharsZ(iter.filename());
uint32_t column;
JS::TaggedColumnNumberZeroOrigin column;
ownedReport.sourceId =
iter.hasScript() ? iter.script()->scriptSource()->id() : 0;
ownedReport.lineno = iter.computeLine(&column);
ownedReport.column = FixupMaybeWASMColumnForDisplay(column);
ownedReport.column = column.oneOriginValue();
ownedReport.isMuted = iter.mutedErrors();
}
}
Expand Down
12 changes: 8 additions & 4 deletions js/src/vm/ErrorObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "js/CallNonGenericMethod.h"
#include "js/CharacterEncoding.h" // JS::ConstUTF8CharsZ
#include "js/Class.h"
#include "js/ColumnNumber.h" // JS::ColumnNumberOneOrigin, JS::TaggedColumnNumberZeroOrigin
#include "js/Conversions.h"
#include "js/ErrorReport.h"
#include "js/friend/ErrorMessages.h" // js::GetErrorMessage, JSMSG_*
Expand Down Expand Up @@ -258,14 +259,16 @@ static ErrorObject* CreateErrorObject(JSContext* cx, const CallArgs& args,
return nullptr;
}

uint32_t lineNumber, columnNumber = 0;
uint32_t lineNumber;
JS::ColumnNumberOneOrigin columnNumber;
if (!hasOptions && args.length() > messageArg + 2) {
if (!ToUint32(cx, args[messageArg + 2], &lineNumber)) {
return nullptr;
}
} else {
lineNumber = iter.done() ? 0 : iter.computeLine(&columnNumber);
columnNumber = FixupMaybeWASMColumnForDisplay(columnNumber);
JS::TaggedColumnNumberZeroOrigin tmp;
lineNumber = iter.done() ? 0 : iter.computeLine(&tmp);
columnNumber = JS::ColumnNumberOneOrigin(tmp.oneOriginValue());
}

RootedObject stack(cx);
Expand All @@ -274,7 +277,8 @@ static ErrorObject* CreateErrorObject(JSContext* cx, const CallArgs& args,
}

return ErrorObject::create(cx, exnType, stack, fileName, sourceId, lineNumber,
columnNumber, nullptr, message, cause, proto);
columnNumber.oneOriginValue(), nullptr, message,
cause, proto);
}

static bool Error(JSContext* cx, unsigned argc, Value* vp) {
Expand Down
15 changes: 7 additions & 8 deletions js/src/vm/ErrorReporting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@

#include "frontend/FrontendContext.h" // AutoReportFrontendContext
#include "js/CharacterEncoding.h" // JS::ConstUTF8CharsZ
#include "js/ColumnNumber.h" // JS::ColumnNumberZeroOrigin
#include "js/ErrorReport.h" // JSErrorBase
#include "js/friend/ErrorMessages.h" // js::GetErrorMessage, JSMSG_*
#include "js/Printf.h" // JS_vsmprintf
#include "js/Warnings.h" // JS::WarningReporter
#include "js/ColumnNumber.h" // JS::ColumnNumberZeroOrigin, JS::TaggedColumnNumberZeroOrigin
#include "js/ErrorReport.h" // JSErrorBase
#include "js/friend/ErrorMessages.h" // js::GetErrorMessage, JSMSG_*
#include "js/Printf.h" // JS_vsmprintf
#include "js/Warnings.h" // JS::WarningReporter
#include "vm/FrameIter.h"
#include "vm/GlobalObject.h"
#include "vm/JSContext.h"
#include "vm/SavedStacks.h" // FixupMaybeWASMColumnForDisplay

using namespace js;

Expand Down Expand Up @@ -197,9 +196,9 @@ static void PopulateReportBlame(JSContext* cx, JSErrorReport* report) {
if (iter.hasScript()) {
report->sourceId = iter.script()->scriptSource()->id();
}
uint32_t column;
JS::TaggedColumnNumberZeroOrigin column;
report->lineno = iter.computeLine(&column);
report->column = FixupMaybeWASMColumnForDisplay(column);
report->column = column.oneOriginValue();
report->isMuted = iter.mutedErrors();
}

Expand Down
20 changes: 14 additions & 6 deletions js/src/vm/FrameIter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
#include "jit/BaselineFrame.h" // js::jit::BaselineFrame
#include "jit/JitFrames.h" // js::jit::EnsureUnwoundJitExitFrame
#include "jit/JSJitFrameIter.h" // js::jit::{FrameType,InlineFrameIterator,JSJitFrameIter,MaybeReadFallback,SnapshotIterator}
#include "js/GCAPI.h" // JS::AutoSuppressGCAnalysis
#include "js/Principals.h" // JSSubsumesOp
#include "js/RootingAPI.h" // JS::Rooted
#include "vm/Activation.h" // js::Activation{,Iterator}
#include "js/ColumnNumber.h" // JS::LimitedColumnNumberZeroOrigin, JS::TaggedColumnNumberZeroOrigin
#include "js/GCAPI.h" // JS::AutoSuppressGCAnalysis
#include "js/Principals.h" // JSSubsumesOp
#include "js/RootingAPI.h" // JS::Rooted
#include "vm/Activation.h" // js::Activation{,Iterator}
#include "vm/EnvironmentObject.h" // js::CallObject
#include "vm/JitActivation.h" // js::jit::JitActivation
#include "vm/JSContext.h" // JSContext
Expand Down Expand Up @@ -618,7 +619,8 @@ const char16_t* FrameIter::displayURL() const {
MOZ_CRASH("Unexpected state");
}

unsigned FrameIter::computeLine(uint32_t* column) const {
unsigned FrameIter::computeLine(
JS::TaggedColumnNumberZeroOrigin* column) const {
switch (data_.state_) {
case DONE:
break;
Expand All @@ -627,7 +629,13 @@ unsigned FrameIter::computeLine(uint32_t* column) const {
if (isWasm()) {
return wasmFrame().computeLine(column);
}
return PCToLineNumber(script(), pc(), column);
unsigned columnNumber;
unsigned lineNumber = PCToLineNumber(script(), pc(), &columnNumber);
if (column) {
*column = JS::TaggedColumnNumberZeroOrigin(
JS::LimitedColumnNumberZeroOrigin(columnNumber));
}
return lineNumber;
}

MOZ_CRASH("Unexpected state");
Expand Down
4 changes: 3 additions & 1 deletion js/src/vm/FrameIter.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "jstypes.h" // JS_PUBLIC_API

#include "jit/JSJitFrameIter.h" // js::jit::{InlineFrameIterator,JSJitFrameIter}
#include "js/ColumnNumber.h" // JS::TaggedColumnNumberZeroOrigin
#include "js/RootingAPI.h" // JS::Handle, JS::Rooted
#include "js/TypeDecls.h" // jsbytecode, JSContext, JSAtom, JSFunction, JSObject, JSScript
#include "js/Value.h" // JS::Value
Expand Down Expand Up @@ -283,7 +284,8 @@ class FrameIter {
ScriptSource* scriptSource() const;
const char* filename() const;
const char16_t* displayURL() const;
unsigned computeLine(uint32_t* column = nullptr) const;
unsigned computeLine(
JS::TaggedColumnNumberZeroOrigin* column = nullptr) const;
JSAtom* maybeFunctionDisplayAtom() const;
bool mutedErrors() const;

Expand Down
9 changes: 5 additions & 4 deletions js/src/vm/SavedFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "mozilla/Attributes.h"

#include "gc/Policy.h"
#include "js/ColumnNumber.h" // JS::TaggedColumnNumberOneOrigin
#include "js/GCHashTable.h"
#include "js/Principals.h"
#include "js/UbiNode.h"
Expand Down Expand Up @@ -50,8 +51,8 @@ class SavedFrame : public NativeObject {
uint32_t getSourceId();
// Line number (1-origin).
uint32_t getLine();
// Column number in UTF-16 code units (1-origin).
uint32_t getColumn();
// Column number in UTF-16 code units.
JS::TaggedColumnNumberOneOrigin getColumn();
JSAtom* getFunctionDisplayName();
JSAtom* getAsyncCause();
SavedFrame* getParent() const;
Expand Down Expand Up @@ -123,7 +124,7 @@ class SavedFrame : public NativeObject {
void initSource(JSAtom* source);
void initSourceId(uint32_t id);
void initLine(uint32_t line);
void initColumn(uint32_t column);
void initColumn(JS::TaggedColumnNumberOneOrigin column);
void initFunctionDisplayName(JSAtom* maybeName);
void initAsyncCause(JSAtom* maybeCause);
void initParent(SavedFrame* maybeParent);
Expand Down Expand Up @@ -260,7 +261,7 @@ class ConcreteStackFrame<SavedFrame> : public BaseStackFrame {

StackFrame parent() const override { return get().getParent(); }
uint32_t line() const override { return get().getLine(); }
uint32_t column() const override { return get().getColumn(); }
uint32_t column() const override { return get().getColumn().rawValue(); }

AtomOrTwoByteChars source() const override {
auto source = get().getSource();
Expand Down
Loading

0 comments on commit 7bb2ff0

Please sign in to comment.