Handle a missing -P/--preinclude file the same as an INCLUDE (#1873)

include/asm/fstack.hpp

@@ -63,7 +63,7 @@ MacroArgs *fstk_GetCurrentMacroArgs();
 void fstk_AddIncludePath(std::string const &path);
 void fstk_AddPreIncludeFile(std::string const &path);
 std::optional<std::string> fstk_FindFile(std::string const &path);
-bool fstk_FileError(std::string const &path, char const *functionName);
+bool fstk_FileError(std::string const &path, char const *description);
 bool fstk_FailedOnMissingInclude();
 
 bool yywrap();
@@ -84,6 +84,6 @@ void fstk_RunFor(
 bool fstk_Break();
 
 void fstk_NewRecursionDepth(size_t newDepth);
-void fstk_Init(std::string const &mainPath);
+bool fstk_Init(std::string const &mainPath);
 
 #endif // RGBDS_ASM_FSTACK_HPP

src/asm/actions.cpp

@@ -139,7 +139,7 @@ std::optional<std::string> act_ReadFile(std::string const &name, uint32_t maxLen
 		file = fopen(fullPath->c_str(), "rb");
 	}
 	if (!file) {
-		if (fstk_FileError(name, "READFILE")) {
+		if (fstk_FileError(name, "`READFILE`")) {
 			// If `fstk_FileError` returned true due to `-MG`, we should abort due to a
 			// missing file, so return `std::nullopt`, which tells the caller to `YYACCEPT`
 			return std::nullopt;

src/asm/fstack.cpp

@@ -191,10 +191,14 @@ std::optional<std::string> fstk_FindFile(std::string const &path) {
 		}
 	}
 
-	errno = ENOENT;
 	if (options.missingIncludeState != INC_ERROR) {
 		printDep(path);
 	}
+
+	// Set `errno` as if `fopen` had failed on a nonexistent file.
+	// This allows a subsequent `fstk_FileError` to report correctly with `strerror`.
+	errno = ENOENT;
+
 	return std::nullopt;
 }
 
@@ -351,17 +355,17 @@ static Context &
 	return context;
 }
 
-bool fstk_FileError(std::string const &path, char const *functionName) {
+bool fstk_FileError(std::string const &path, char const *description) {
 	if (options.missingIncludeState == INC_ERROR) {
-		error("Error opening `%s` file \"%s\": %s", functionName, path.c_str(), strerror(errno));
+		error("Error opening %s file \"%s\": %s", description, path.c_str(), strerror(errno));
 	} else {
 		failedOnMissingInclude = true;
 		// LCOV_EXCL_START
 		if (options.missingIncludeState == GEN_EXIT) {
 			verbosePrint(
 			    VERB_NOTICE,
-			    "Aborting due to '-MG' on `%s` file \"%s\": %s\n",
+			    "Aborting due to '-MG' on %s file \"%s\": %s\n",
-			    functionName,
+			    description,
 			    path.c_str(),
 			    strerror(errno)
 			);
@@ -382,7 +386,7 @@ bool fstk_RunInclude(std::string const &path, bool isQuiet) {
 		newFileContext(*fullPath, isQuiet, false);
 		return false;
 	}
-	return fstk_FileError(path, "INCLUDE");
+	return fstk_FileError(path, "`INCLUDE`");
 }
 
 void fstk_RunMacro(
@@ -490,14 +494,16 @@ void fstk_NewRecursionDepth(size_t newDepth) {
 	options.maxRecursionDepth = newDepth;
 }
 
-void fstk_Init(std::string const &mainPath) {
+bool fstk_Init(std::string const &mainPath) {
 	newFileContext(mainPath, false, true);
 
 	for (std::string const &name : preIncludeNames) {
 		if (std::optional<std::string> fullPath = fstk_FindFile(name); fullPath) {
 			newFileContext(*fullPath, false, false);
-		} else {
+		} else if (fstk_FileError(name, "pre-included")) {
-			error("Error reading pre-included file \"%s\": %s", name.c_str(), strerror(errno));
+			return false;
 		}
 	}
+
+	return true;
 }

src/asm/main.cpp

@@ -558,11 +558,8 @@ int main(int argc, char *argv[]) {
 
 	charmap_New(DEFAULT_CHARMAP_NAME, nullptr);
 
-	// Init lexer and file stack, providing file info
+	// Init lexer and file stack, and parse (`yy::parser` is auto-generated from `parser.y`)
-	fstk_Init(*localOptions.inputFileName);
+	if (yy::parser parser; fstk_Init(*localOptions.inputFileName) && parser.parse() != 0) {
-
-	// Perform parse (`yy::parser` is auto-generated from `parser.y`)
-	if (yy::parser parser; parser.parse() != 0) {
 		// Exited due to YYABORT or YYNOMEM
 		fatal("Unrecoverable error while parsing"); // LCOV_EXCL_LINE
 	}

src/asm/section.cpp

@@ -929,7 +929,7 @@ bool sect_BinaryFile(std::string const &name, uint32_t startPos) {
 		file = fopen(fullPath->c_str(), "rb");
 	}
 	if (!file) {
-		return fstk_FileError(name, "INCBIN");
+		return fstk_FileError(name, "`INCBIN`");
 	}
 	Defer closeFile{[&] { fclose(file); }};
 
@@ -984,7 +984,7 @@ bool sect_BinaryFileSlice(std::string const &name, uint32_t startPos, uint32_t l
 		file = fopen(fullPath->c_str(), "rb");
 	}
 	if (!file) {
-		return fstk_FileError(name, "INCBIN");
+		return fstk_FileError(name, "`INCBIN`");
 	}
 	Defer closeFile{[&] { fclose(file); }};
 

src/link/main.cpp

@@ -446,11 +446,9 @@ int main(int argc, char *argv[]) {
 	if (localOptions.linkerScriptName) {
 		verbosePrint(VERB_NOTICE, "Reading linker script...\n");
 
-		if (lexer_Init(*localOptions.linkerScriptName)) {
+		if (yy::parser parser; lexer_Init(*localOptions.linkerScriptName) && parser.parse() != 0) {
-			if (yy::parser parser; parser.parse() != 0) {
+			// Exited due to YYABORT or YYNOMEM
-				// Exited due to YYABORT or YYNOMEM
+			fatal("Unrecoverable error while reading linker script"); // LCOV_EXCL_LINE
-				fatal("Unrecoverable error while reading linker script"); // LCOV_EXCL_LINE
-			}
 		}
 
 		// If the linker script produced any errors, some sections may be in an invalid state

test/asm/continues-after-missing-include/a.flags

@@ -0,0 +1 @@
+-MG -MC

test/asm/continues-after-missing-preinclude/a.asm

@@ -0,0 +1,13 @@
+PUSHC
+PUSHO
+PUSHS
+SECTION "test", WRAM0
+UNION
+INCLUDE "nonexistent1.inc"
+WARN "still going!"
+INCLUDE "nonexistent2.inc"
+WARN "and going!"
+ENDU
+POPS
+POPO
+POPC

test/asm/continues-after-missing-preinclude/a.err

@@ -0,0 +1,4 @@
+warning: still going! [-Wuser]
+    at continues-after-missing-preinclude/a.asm(7)
+warning: and going! [-Wuser]
+    at continues-after-missing-preinclude/a.asm(9)

test/asm/continues-after-missing-preinclude/a.flags

@@ -0,0 +1 @@
+-MC -P nonexistent-pre.inc

test/asm/continues-after-missing-preinclude/a.out

@@ -0,0 +1,4 @@
+a.o: continues-after-missing-preinclude/a.asm
+a.o: nonexistent-pre.inc
+a.o: nonexistent1.inc
+a.o: nonexistent2.inc

test/asm/errors-after-missing-include/a.out

@@ -1,2 +0,0 @@
-a.o: errors-after-missing-include/a.asm
-a.o: does not exist

test/asm/exits-after-missing-include/a.flags

@@ -0,0 +1 @@
+-MG

test/asm/exits-after-missing-include/a.out

@@ -0,0 +1,2 @@
+a.o: exits-after-missing-include/a.asm
+a.o: does not exist

test/asm/exits-after-missing-preinclude/a.asm

@@ -0,0 +1,12 @@
+PUSHC
+PUSHO
+PUSHS
+SECTION "test", WRAM0
+UNION
+INCLUDE "never reaches here"
+/*
+ENDU
+POPS
+POPO
+POPC
+*/

test/asm/exits-after-missing-preinclude/a.flags

@@ -0,0 +1 @@
+-MG -P nonexistent-pre.inc

test/asm/exits-after-missing-preinclude/a.out

@@ -0,0 +1,2 @@
+a.o: exits-after-missing-preinclude/a.asm
+a.o: nonexistent-pre.inc

test/asm/include-slash.err

@@ -1,3 +1,3 @@
-error: Error reading pre-included file "include-slash-nonexist.inc": No such file or directory
+error: Error opening pre-included file "include-slash-nonexist.inc": No such file or directory
     at include-slash.asm(0)
 Assembly aborted with 1 error!

test/asm/nonexist-preinclude.asm

@@ -0,0 +1 @@
+INCLUDE "nonexist-include.inc"

test/asm/nonexist-preinclude.err

@@ -0,0 +1,5 @@
+error: Error opening pre-included file "nonexistent-pre.inc": No such file or directory
+    at nonexist-preinclude.asm(0)
+error: Error opening `INCLUDE` file "nonexist-include.inc": No such file or directory
+    at nonexist-preinclude.asm(1)
+Assembly aborted with 2 errors!

test/asm/nonexist-preinclude.flags

@@ -0,0 +1 @@
+-P nonexistent-pre.inc

test/asm/test.sh

@@ -163,11 +163,14 @@ done
 
 evaluateDepTest () {
 	i="$1"
-	RGBASMFLAGS="-Weverything -Bcollapse -M - $2"
+	RGBASMFLAGS="-Weverything -Bcollapse -M -"
+	if [ -f "$i/a.flags" ]; then
+		RGBASMFLAGS="$RGBASMFLAGS @$i/a.flags"
+	fi
 	# Piping the .asm file to rgbasm would not make sense for dependency generation,
 	# so just test the normal variant
 	(( tests++ ))
-	echo "${bold}${green}${i%.asm}...${rescolors}${resbold}"
+	echo "${bold}${green}${i}...${rescolors}${resbold}"
 	"$RGBASM" $RGBASMFLAGS -o "$o" "$i"/a.asm >"$output" 2>"$errput"
 
 	fixed_output="$input"
@@ -192,8 +195,10 @@ evaluateDepTest () {
 		(( failed++ ))
 	fi
 }
-evaluateDepTest "continues-after-missing-include" "-MG -MC"
+evaluateDepTest "continues-after-missing-include"
-evaluateDepTest "errors-after-missing-include" "-MG"
+evaluateDepTest "exits-after-missing-include"
+evaluateDepTest "continues-after-missing-preinclude"
+evaluateDepTest "exits-after-missing-preinclude"
 
 i="state-file"
 if which cygpath &>/dev/null; then