Skip to content

Commit 84c8eb4

Browse files
committed
Avoid double-reporting the filename in JS library errors/warnings
As part of this I also avoid converting JS libraries path to absolute paths where possible. This means that errors and warnings show the actual pathname passed by the users, and not some absolute path. This makes writing tests for the double-reporting issue easier, and makes the errors/warnings easier to read (IMHO).
1 parent fa5b2aa commit 84c8eb4

File tree

3 files changed

+27
-25
lines changed

3 files changed

+27
-25
lines changed

src/parseTools.mjs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ export function preprocess(filename) {
152152
}
153153
const absPath = findIncludeFile(includeFile, path.dirname(filename));
154154
if (!absPath) {
155-
error(`${filename}:${i + 1}: file not found: ${includeFile}`);
155+
error(`file not found: ${includeFile}`, i + 1);
156156
continue;
157157
}
158158
const result = preprocess(absPath);
@@ -164,7 +164,7 @@ export function preprocess(filename) {
164164
}
165165
} else if (first === '#else') {
166166
if (showStack.length == 0) {
167-
error(`${filename}:${i + 1}: #else without matching #if`);
167+
error('#else without matching #if', i + 1);
168168
}
169169
const curr = showStack.pop();
170170
if (curr == IGNORE) {
@@ -174,23 +174,21 @@ export function preprocess(filename) {
174174
}
175175
} else if (first === '#endif') {
176176
if (showStack.length == 0) {
177-
error(`${filename}:${i + 1}: #endif without matching #if`);
177+
error('#endif without matching #if', i + 1);
178178
}
179179
showStack.pop();
180180
} else if (first === '#warning') {
181181
if (showCurrentLine()) {
182-
warn(
183-
`${filename}:${i + 1}: #warning ${trimmed.substring(trimmed.indexOf(' ')).trim()}`,
184-
);
182+
warn(`#warning ${trimmed.substring(trimmed.indexOf(' ')).trim()}`, i + 1);
185183
}
186184
} else if (first === '#error') {
187185
if (showCurrentLine()) {
188-
error(`${filename}:${i + 1}: #error ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
186+
error(`#error ${trimmed.substring(trimmed.indexOf(' ')).trim()}`, i + 1);
189187
}
190188
} else if (first === '#preprocess') {
191189
// Do nothing
192190
} else {
193-
error(`${filename}:${i + 1}: Unknown preprocessor directive ${first}`);
191+
error(`Unknown preprocessor directive ${first}`, i + 1);
194192
}
195193
} else {
196194
if (showCurrentLine()) {

src/utility.mjs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,19 @@ export function popCurrentFile() {
5656
currentFile.pop();
5757
}
5858

59-
function errorPrefix() {
60-
if (currentFile.length > 0) {
61-
return currentFile[currentFile.length - 1] + ': ';
59+
function errorPrefix(lineNo) {
60+
if (!currentFile.length) return '';
61+
const filename = currentFile[currentFile.length - 1];
62+
if (lineNo) {
63+
return `${filename}:${lineNo}: `;
6264
} else {
63-
return '';
65+
return `${filename}: `;
6466
}
6567
}
6668

67-
export function warn(msg) {
69+
export function warn(msg, lineNo) {
6870
warnings = true;
69-
printErr(`warning: ${errorPrefix()}${msg}`);
71+
printErr(`warning: ${errorPrefix(lineNo)}${msg}`);
7072
}
7173

7274
const seenWarnings = new Set();
@@ -84,9 +86,9 @@ export function errorOccured() {
8486
return abortExecution;
8587
}
8688

87-
export function error(msg) {
89+
export function error(msg, lineNo) {
8890
abortExecution = true;
89-
printErr(`error: ${errorPrefix()}${msg}`);
91+
printErr(`error: ${errorPrefix(lineNo)}${msg}`);
9092
}
9193

9294
function range(size) {

test/test_other.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4915,24 +4915,26 @@ def test_jslib_search_path(self):
49154915

49164916
# Tests using the #warning directive in JS library files
49174917
def test_jslib_warnings(self):
4918-
proc = self.run_process([EMCC, test_file('hello_world.c'), '--js-library', test_file('warning_in_js_libraries.js')], stdout=PIPE, stderr=PIPE)
4918+
shutil.copy(test_file('warning_in_js_libraries.js'), '.')
4919+
proc = self.run_process([EMCC, test_file('hello_world.c'), '--js-library', 'warning_in_js_libraries.js'], stdout=PIPE, stderr=PIPE)
49194920
self.assertNotContained('This warning should not be present!', proc.stderr)
4920-
self.assertContained('warning_in_js_libraries.js:5: #warning This is a warning string!', proc.stderr)
4921-
self.assertContained('warning_in_js_libraries.js:7: #warning This is a second warning string!', proc.stderr)
4921+
self.assertContained('warning: warning_in_js_libraries.js:5: #warning This is a warning string!', proc.stderr)
4922+
self.assertContained('warning: warning_in_js_libraries.js:7: #warning This is a second warning string!', proc.stderr)
49224923
self.assertContained('emcc: warning: warnings in JS library compilation [-Wjs-compiler]', proc.stderr)
49234924

4924-
err = self.expect_fail([EMCC, test_file('hello_world.c'), '--js-library', test_file('warning_in_js_libraries.js'), '-Werror'])
4925+
err = self.expect_fail([EMCC, test_file('hello_world.c'), '--js-library', 'warning_in_js_libraries.js', '-Werror'])
49254926
self.assertNotContained('This warning should not be present!', err)
4926-
self.assertContained('warning_in_js_libraries.js:5: #warning This is a warning string!', err)
4927-
self.assertContained('warning_in_js_libraries.js:7: #warning This is a second warning string!', err)
4927+
self.assertContained('warning: warning_in_js_libraries.js:5: #warning This is a warning string!', err)
4928+
self.assertContained('warning: warning_in_js_libraries.js:7: #warning This is a second warning string!', err)
49284929
self.assertContained('emcc: error: warnings in JS library compilation [-Wjs-compiler] [-Werror]', err)
49294930

49304931
# Tests using the #error directive in JS library files
49314932
def test_jslib_errors(self):
4932-
err = self.expect_fail([EMCC, test_file('hello_world.c'), '--js-library', test_file('error_in_js_libraries.js')])
4933+
shutil.copy(test_file('error_in_js_libraries.js'), '.')
4934+
err = self.expect_fail([EMCC, test_file('hello_world.c'), '--js-library', 'error_in_js_libraries.js'])
49334935
self.assertNotContained('This error should not be present!', err)
4934-
self.assertContained('error_in_js_libraries.js:5: #error This is an error string!', err)
4935-
self.assertContained('error_in_js_libraries.js:7: #error This is a second error string!', err)
4936+
self.assertContained('error: error_in_js_libraries.js:5: #error This is an error string!', err)
4937+
self.assertContained('error: error_in_js_libraries.js:7: #error This is a second error string!', err)
49364938

49374939
def test_jslib_include(self):
49384940
create_file('inc.js', '''

0 commit comments

Comments
 (0)