[gjs/wip/ptomato/develop: 7/9] coverage: Use frame callee script data to get line
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs/wip/ptomato/develop: 7/9] coverage: Use frame callee script data to get line
- Date: Mon, 25 Sep 2017 06:30:51 +0000 (UTC)
commit 2ab05026af1b4cc0ca77a773a5deb7bb5673dadc
Author: Philip Chimento <philip chimento gmail com>
Date: Sat Sep 23 22:02:46 2017 -0700
coverage: Use frame callee script data to get line
Previously, the coverage code had to deal with the entry point of a
function (and therefore a frame) being on a different line than the
definition of the function. So, when entering a debugger frame, we had to
walk backwards through the source until a matching function definition
was encountered. This was slow and complicated.
Now, we can get the line where the function was defined by accessing the
Debugger.Script object belonging to the frame's callee. This allows us to
get rid of the walking-backwards code, and stop keeping an array of lines
with known functions.
Unfortunately even though the Reflect.parse API now gives column
information, you still can't get the column where a function was defined
from the Debugger.Script object. So we still have to disambiguate
functions defined on the same line using their number of arguments.
installed-tests/js/testCoverage.js | 40 ++--------------
modules/_bootstrap/coverage.js | 89 +++++++----------------------------
test/gjs-test-coverage.cpp | 1 +
3 files changed, 25 insertions(+), 105 deletions(-)
---
diff --git a/installed-tests/js/testCoverage.js b/installed-tests/js/testCoverage.js
index 56d878f..e1def31 100644
--- a/installed-tests/js/testCoverage.js
+++ b/installed-tests/js/testCoverage.js
@@ -868,18 +868,6 @@ describe('Coverage', function () {
'Function coverage will be incomplete.');
});
- it('populates a known functions array', function () {
- let functions = [
- { line: 1 },
- { line: 2 }
- ];
-
- let knownFunctionsArray = Coverage._populateKnownFunctions(functions, 4);
-
- expect(knownFunctionsArray)
- .toEqual([undefined, true, true, undefined, undefined]);
- });
-
it('converts function counters to an array', function () {
let functionsMap = {
'(anonymous)': {
@@ -916,7 +904,7 @@ describe('Coverage.incrementFunctionCounters', function () {
line: 1,
n_params: 0 }
]);
- Coverage._incrementFunctionCounters(functionCounters, null, 'f', 1, 0);
+ Coverage._incrementFunctionCounters(functionCounters, 'f', 1, 0);
expect(functionCounters.f['1']['0'].hitCount).toEqual(1);
});
@@ -930,8 +918,8 @@ describe('Coverage.incrementFunctionCounters', function () {
line: 2,
n_params: 0 }
]);
- Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 0);
- Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 2, 0);
+ Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 0);
+ Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 2, 0);
expect(functionCounters['(anonymous)']['1']['0'].hitCount).toEqual(1);
expect(functionCounters['(anonymous)']['2']['0'].hitCount).toEqual(1);
@@ -946,8 +934,8 @@ describe('Coverage.incrementFunctionCounters', function () {
line: 1,
n_params: 1 }
]);
- Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 0);
- Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 1);
+ Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 0);
+ Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 1);
expect(functionCounters['(anonymous)']['1']['0'].hitCount).toEqual(1);
expect(functionCounters['(anonymous)']['1']['1'].hitCount).toEqual(1);
@@ -966,30 +954,15 @@ describe('Coverage.incrementFunctionCounters', function () {
/* Eg, we called the function with 3 params with just two arguments. We
* should be able to work out that we probably intended to call the
* latter function as opposed to the former. */
- Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 2);
+ Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 2);
expect(functionCounters['(anonymous)']['1']['0'].hitCount).toEqual(0);
expect(functionCounters['(anonymous)']['1']['3'].hitCount).toEqual(1);
});
- it('increments for function on earlier start line', function () {
- let ast = Reflect.parse('function name() {}');
- let detectedFunctions = Coverage.functionsForAST(ast);
- let knownFunctionsArray = Coverage._populateKnownFunctions(detectedFunctions, 3);
- let functionCounters = Coverage._functionsToFunctionCounters('script',
- detectedFunctions);
-
- /* We're entering at line two, but the function definition was actually
- * at line one */
- Coverage._incrementFunctionCounters(functionCounters, knownFunctionsArray, 'name', 2, 0);
-
- expect(functionCounters.name['1']['0'].hitCount).toEqual(1);
- });
-
it('throws an error on unexpected function', function () {
let ast = Reflect.parse('function name() {}');
let detectedFunctions = Coverage.functionsForAST(ast);
- let knownFunctionsArray = Coverage._populateKnownFunctions(detectedFunctions, 3);
let functionCounters = Coverage._functionsToFunctionCounters('script',
detectedFunctions);
@@ -997,7 +970,6 @@ describe('Coverage.incrementFunctionCounters', function () {
* at line one */
expect(() => {
Coverage._incrementFunctionCounters(functionCounters,
- knownFunctionsArray,
'doesnotexist',
2,
0);
diff --git a/modules/_bootstrap/coverage.js b/modules/_bootstrap/coverage.js
index d5c05a8..7698a8d 100644
--- a/modules/_bootstrap/coverage.js
+++ b/modules/_bootstrap/coverage.js
@@ -196,16 +196,16 @@ function _getFunctionKeyFromReflectedFunction(node) {
return [name, line, n_params].join(':');
}
-/* Unfortunately, the Reflect API doesn't give us enough information to
- * uniquely identify a function. A function might be anonymous, in which
- * case the JS engine uses some heurisitics to get a unique string identifier
- * but that isn't available to us here.
+/* Unfortunately, the Debugger API doesn't give us enough information to
+ * uniquely identify a function when we enter its frame. A function might be
+ * anonymous, in which case the JS engine uses some heurisitics to get a
+ * unique string identifier but that isn't available to us here.
*
* There's also the edge-case where functions with the same name might be
* defined within the same scope, or multiple anonymous functions might
* be defined on the same line. In that case, it will look like we entered
* the same function multiple times since we can't get column information
- * from the engine-side.
+ * from the debugger frame.
*
* For instance:
*
@@ -524,16 +524,6 @@ function _functionsToFunctionCounters(script, functions) {
return functionCounters;
}
-function _populateKnownFunctions(functions, nLines) {
- let knownFunctions = new Array(nLines + 1);
-
- functions.forEach(function(func) {
- knownFunctions[func.line] = true;
- });
-
- return knownFunctions;
-}
-
function _identifyFunctionCounterInLinePartForDescription(linePart,
nArgs) {
/* There is only one potential option for this line. We might have been
@@ -564,29 +554,6 @@ function _identifyFunctionCounterInLinePartForDescription(linePart,
return linePart[String(closest)];
}
-function _identifyFunctionCounterForDescription(functionCounters,
- name,
- line,
- nArgs) {
- let candidateCounter = functionCounters[name];
-
- if (candidateCounter === undefined)
- return null;
-
- if (Object.getOwnPropertyNames(candidateCounter).length === 1) {
- let linePart = candidateCounter[Object.getOwnPropertyNames(candidateCounter)[0]];
- return _identifyFunctionCounterInLinePartForDescription(linePart, nArgs);
- }
-
- let linePart = functionCounters[name][line];
-
- if (linePart === undefined) {
- return null;
- }
-
- return _identifyFunctionCounterInLinePartForDescription(linePart, nArgs);
-}
-
/**
* _incrementFunctionCounters
*
@@ -594,38 +561,19 @@ function _identifyFunctionCounterForDescription(functionCounters,
* {
* "key" : { line, hitCount }
* }
- * linesWithKnownFunctions: An array of either "true" or undefined, with true set to
- * each element corresponding to a line that we know has a function on it.
* name: The name of the function or "(anonymous)" if it has no name
- * line: The line at which execution first started on this function.
+ * line: The line at which this function is defined.
* nArgs: The number of arguments this function has.
*/
-function _incrementFunctionCounters(functionCounters,
- linesWithKnownFunctions,
- name,
- line,
- nArgs) {
- let functionCountersForKey = _identifyFunctionCounterForDescription(functionCounters,
- name,
- line,
- nArgs);
-
- /* Its possible that the JS Engine might enter a funciton
- * at an executable line which is a little bit past the
- * actual definition. Roll backwards until we reach the
- * last known function definition line which we kept
- * track of earlier to see if we can find this function first */
- if (functionCountersForKey === null) {
- do {
- --line;
- functionCountersForKey = _identifyFunctionCounterForDescription(functionCounters,
- name,
- line,
- nArgs);
- } while (linesWithKnownFunctions[line] !== true && line > 0);
- }
-
- if (functionCountersForKey !== null) {
+function _incrementFunctionCounters(functionCounters, name, line, nArgs) {
+ let functionCountersForKey;
+ let linePart = functionCounters[name][line];
+ if (linePart) {
+ functionCountersForKey =
+ _identifyFunctionCounterInLinePartForDescription(linePart, nArgs);
+ }
+
+ if (functionCountersForKey) {
functionCountersForKey.hitCount++;
} else {
let functionKey = [name, line, nArgs].join(':');
@@ -768,7 +716,6 @@ function _fetchCountersFromCache(filename, cache, nLines) {
expressionCounters: _expressionLinesToCounters(cache_for_file.lines, nLines),
branchCounters: _branchesToBranchCounters(cache_for_file.branches, nLines),
functionCounters: _functionsToFunctionCounters(filename, functions),
- linesWithKnownFunctions: _populateKnownFunctions(functions, nLines),
nLines: nLines
};
}
@@ -787,7 +734,6 @@ function _fetchCountersFromReflection(filename, contents, nLines) {
expressionCounters: _expressionLinesToCounters(expressionLinesForAST(reflection), nLines),
branchCounters: _branchesToBranchCounters(branchesForAST(reflection), nLines),
functionCounters: _functionsToFunctionCounters(filename, functions),
- linesWithKnownFunctions: _populateKnownFunctions(functions, nLines),
nLines: nLines
};
}
@@ -928,17 +874,18 @@ var CoverageStatistics = class CoverageStatistics {
/* Log function calls */
if (frame.callee !== null && frame.callee.callable) {
let name = frame.callee.name || '(anonymous)';
- let {lineNumber} = frame.script.getOffsetLocation(frame.offset);
+ // FIXME: https://bugzilla.mozilla.org/show_bug.cgi?id=901138
+ // There should be a Debugger.Script.prototype.startColumn
+ let {startLine} = frame.script;
let nArgs = frame.callee.parameterNames.length;
try {
_incrementFunctionCounters(statistics.functionCounters,
- statistics.linesWithKnownFunctions, name, lineNumber,
- nArgs);
+ name, startLine, nArgs);
} catch (e) {
/* Something bad happened. Log the exception and delete
* statistics for this file */
- _logExceptionAndReset(e, name, lineNumber);
+ _logExceptionAndReset(e, name, startLine);
return undefined;
}
}
diff --git a/test/gjs-test-coverage.cpp b/test/gjs-test-coverage.cpp
index 10d08a7..c7b2824 100644
--- a/test/gjs-test-coverage.cpp
+++ b/test/gjs-test-coverage.cpp
@@ -1859,6 +1859,7 @@ test_coverage_cache_invalidation(gpointer fixture_data,
const gsize expected_len = G_N_ELEMENTS(expected);
const char *record = line_starting_with(coverage_data_contents, "SF:");
+ g_assert_nonnull(record);
g_assert(check_coverage_data_for_source_file(expected, expected_len, record));
g_free(script_output_path);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
