Make ENDL optional like ENDSECTION (#1538)

Add warning for `LOAD` without `ENDL`

contrib/bash_compl/_rgbasm.bash

@@ -192,6 +192,7 @@ _rgbasm_completions() {
 				shift-amount
 				truncation
 				unmapped-char
+				unterminated-load
 				user
 				all
 				extra

contrib/zsh_compl/_rgbasm

@@ -26,6 +26,7 @@ _rgbasm_warnings() {
 		'shift-amount:Warn when a shift'\''s operand it negative or \> 32'
 		'truncation:Warn when implicit truncation loses bits'
 		'unmapped-char:Warn on unmapped character'
+		'unterminated-load:Warn on LOAD without ENDL'
 		'user:Warn when executing the WARN built-in'
 	)
 	# TODO: handle `no-` and `error=` somehow?

include/asm/section.hpp

@@ -70,7 +70,8 @@ void sect_SetLoadSection(
     SectionSpec const &attrs,
     SectionModifier mod
 );
-void sect_EndLoadSection();
+void sect_EndLoadSection(char const *cause);
+void sect_CheckLoadClosed();
 
 Section *sect_GetSymbolSection();
 uint32_t sect_GetSymbolOffset();

include/asm/warning.hpp

@@ -7,20 +7,21 @@ extern unsigned int nbErrors, maxErrors;
 
 enum WarningID {
 	WARNING_ASSERT,               // Assertions
-	WARNING_BACKWARDS_FOR,        // `for` loop with backwards range
+	WARNING_BACKWARDS_FOR,        // `FOR` loop with backwards range
 	WARNING_BUILTIN_ARG,          // Invalid args to builtins
 	WARNING_CHARMAP_REDEF,        // Charmap entry re-definition
-	WARNING_DIV,                  // Division undefined behavior
+	WARNING_DIV,                  // Undefined division behavior
 	WARNING_EMPTY_DATA_DIRECTIVE, // `db`, `dw` or `dl` directive without data in ROM
 	WARNING_EMPTY_MACRO_ARG,      // Empty macro argument
 	WARNING_EMPTY_STRRPL,         // Empty second argument in `STRRPL`
 	WARNING_LARGE_CONSTANT,       // Constants too large
-	WARNING_MACRO_SHIFT,          // Shift past available arguments in macro
+	WARNING_MACRO_SHIFT,          // `SHIFT` past available arguments in macro
 	WARNING_NESTED_COMMENT,       // Comment-start delimiter in a block comment
-	WARNING_OBSOLETE,             // Obsolete things
-	WARNING_SHIFT,                // Shifting undefined behavior
-	WARNING_SHIFT_AMOUNT,         // Strange shift amount
-	WARNING_USER,                 // User warnings
+	WARNING_OBSOLETE,             // Obsolete/deprecated things
+	WARNING_SHIFT,                // Undefined `SHIFT` behavior
+	WARNING_SHIFT_AMOUNT,         // Strange `SHIFT` amount
+	WARNING_UNTERMINATED_LOAD,    // `LOAD` without `ENDL`
+	WARNING_USER,                 // User-defined `WARN`ings
 
 	NB_PLAIN_WARNINGS,
 

man/rgbasm.1

@@ -370,6 +370,13 @@ only warns if the active charmap is not empty.
 .Fl Wunmapped-char=2
 warns if the active charmap is empty, and/or is not the default charmap
 .Sq main .
+.It Fl Wunterminated-load
+Warn when a
+.Ic LOAD
+block is not terminated by an
+.Ic ENDL .
+This warning is enabled by
+.Fl Wextra .
 .It Fl Wno-user
 Warn when the
 .Ic WARN

man/rgbasm.5

@@ -903,7 +903,7 @@ SECTION "LOAD example", ROMX
 CopyCode:
     ld de, RAMCode
     ld hl, RAMLocation
-    ld c, RAMLocation.end - RAMLocation
+    ld c, RAMCode.end - RAMCode
 \&.loop
     ld a, [de]
     inc de
@@ -927,8 +927,8 @@ RAMLocation:
 
 \&.string
     db "Hello World!\e0"
-\&.end
   ENDL
+\&.end
 .Ed
 .Pp
 A
@@ -938,7 +938,9 @@ block feels similar to a
 declaration because it creates a new one.
 All data and code generated within such a block is placed in the current section like usual, but all labels are created as if they were placed in this newly-created section.
 .Pp
-In the example above, all of the code and data will end up in the "LOAD example" section.
+In the example above, all of the code and data will end up in the
+.Dq LOAD example
+section.
 You will notice the
 .Sq RAMCode
 and
@@ -950,6 +952,19 @@ You cannot nest
 .Ic LOAD
 blocks, nor can you change or stop the current section within them.
 .Pp
+The current
+.Ic LOAD
+block can be ended by using
+.Ic ENDL .
+This directive is only necessary if you want to resume writing code in its containing ROM section.
+Any of
+.Ic LOAD , SECTION , ENDSECTION ,
+or
+.Ic POPS
+will end the current
+.Ic LOAD
+block before performing its own function.
+.Pp
 .Ic LOAD
 blocks can use the
 .Ic UNION

src/asm/main.cpp

@@ -381,6 +381,7 @@ int main(int argc, char *argv[]) {
 		nbErrors = 1;
 
 	sect_CheckUnionClosed();
+	sect_CheckLoadClosed();
 	sect_CheckSizes();
 
 	if (nbErrors != 0)

src/asm/parser.y

@@ -849,7 +849,7 @@ load:
 		sect_SetLoadSection($3, $5, $6, $7, $2);
 	}
 	| POP_ENDL {
-		sect_EndLoadSection();
+		sect_EndLoadSection(nullptr);
 	}
 ;
 

src/asm/section.cpp

@@ -425,14 +425,14 @@ void sect_NewSection(
     SectionSpec const &attrs,
     SectionModifier mod
 ) {
-	if (currentLoadSection)
-		fatalerror("Cannot change the section within a `LOAD` block\n");
-
 	for (SectionStackEntry &entry : sectionStack) {
 		if (entry.section && entry.section->name == name)
 			fatalerror("Section '%s' is already on the stack\n", name.c_str());
 	}
 
+	if (currentLoadSection)
+		sect_EndLoadSection("SECTION");
+
 	Section *sect = getSection(name, type, org, attrs, mod);
 
 	changeSection();
@@ -457,11 +457,6 @@ void sect_SetLoadSection(
 	if (!requireCodeSection())
 		return;
 
-	if (currentLoadSection) {
-		error("`LOAD` blocks cannot be nested\n");
-		return;
-	}
-
 	if (sect_HasData(type)) {
 		error("`LOAD` blocks cannot create a ROM section\n");
 		return;
@@ -472,6 +467,9 @@ void sect_SetLoadSection(
 		return;
 	}
 
+	if (currentLoadSection)
+		sect_EndLoadSection("LOAD");
+
 	Section *sect = getSection(name, type, org, attrs, mod);
 
 	currentLoadLabelScopes = sym_GetCurrentLabelScopes();
@@ -481,7 +479,14 @@ void sect_SetLoadSection(
 	currentLoadSection = sect;
 }
 
-void sect_EndLoadSection() {
+void sect_EndLoadSection(char const *cause) {
+	if (cause)
+		warning(
+		    WARNING_UNTERMINATED_LOAD,
+		    "`LOAD` block without `ENDL` terminated by `%s`\n",
+		    cause
+		);
+
 	if (!currentLoadSection) {
 		error("Found `ENDL` outside of a `LOAD` block\n");
 		return;
@@ -494,6 +499,11 @@ void sect_EndLoadSection() {
 	sym_SetCurrentLabelScopes(currentLoadLabelScopes);
 }
 
+void sect_CheckLoadClosed() {
+	if (currentLoadSection)
+		warning(WARNING_UNTERMINATED_LOAD, "`LOAD` block without `ENDL` terminated by EOF\n");
+}
+
 Section *sect_GetSymbolSection() {
 	return currentLoadSection ? currentLoadSection : currentSection;
 }
@@ -954,7 +964,7 @@ void sect_PopSection() {
 		fatalerror("No entries in the section stack\n");
 
 	if (currentLoadSection)
-		fatalerror("Cannot change the section within a `LOAD` block\n");
+		sect_EndLoadSection("POPS");
 
 	SectionStackEntry entry = sectionStack.front();
 	sectionStack.pop_front();
@@ -972,12 +982,12 @@ void sect_EndSection() {
 	if (!currentSection)
 		fatalerror("Cannot end the section outside of a SECTION\n");
 
-	if (currentLoadSection)
-		fatalerror("Cannot end the section within a `LOAD` block\n");
-
 	if (!currentUnionStack.empty())
 		fatalerror("Cannot end the section within a UNION\n");
 
+	if (currentLoadSection)
+		sect_EndLoadSection("ENDSECTION");
+
 	// Reset the section scope
 	currentSection = nullptr;
 	sym_ResetCurrentLabelScopes();

src/asm/warning.cpp

@@ -56,6 +56,7 @@ static const WarningFlag warningFlags[NB_WARNINGS] = {
     {"obsolete",             LEVEL_DEFAULT   },
     {"shift",                LEVEL_EVERYTHING},
     {"shift-amount",         LEVEL_EVERYTHING},
+    {"unterminated-load",    LEVEL_EXTRA     },
     {"user",                 LEVEL_DEFAULT   },
     // Parametric warnings
     {"numeric-string",       LEVEL_EVERYTHING},

test/asm/endsection-in-load.err

@@ -1,2 +1,5 @@
-FATAL: endsection-in-load.asm(3):
-    Cannot end the section within a `LOAD` block
+warning: endsection-in-load.asm(3): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by `ENDSECTION`
+error: endsection-in-load.asm(4):
+    Found `ENDL` outside of a `LOAD` block
+error: Assembly aborted (1 error)!

test/asm/load-endings.asm

@@ -0,0 +1,51 @@
+MACRO data
+	db SECTION(@), \#
+ENDM
+
+MACRO now_in
+	if strcmp("\1", "nothing")
+		assert !strcmp(SECTION(@), \1)
+	else
+		assert !def(@)
+	endc
+ENDM
+
+now_in nothing
+
+SECTION "A", ROM0
+	now_in "A"
+	data 1
+	LOAD "P", WRAM0
+		now_in "P"
+		data 2
+
+	; LOAD after LOAD
+	LOAD "Q", WRAM0
+		now_in "Q"
+		data 3
+
+; SECTION after LOAD
+SECTION "B", ROM0
+	now_in "B"
+	data 4
+	LOAD "R", WRAM0
+		now_in "R"
+		data 5
+
+		; PUSHS after LOAD
+		PUSHS
+			SECTION "C", ROM0
+				now_in "C"
+				data 6
+				LOAD "S", WRAM0
+					now_in "S"
+					data 7
+
+		; POPS after LOAD
+		POPS
+		now_in "R"
+		data 8
+
+; ENDSECTION after LOAD
+ENDSECTION
+now_in nothing

test/asm/load-endings.err

@@ -0,0 +1,8 @@
+warning: load-endings.asm(23): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by `LOAD`
+warning: load-endings.asm(28): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by `SECTION`
+warning: load-endings.asm(45): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by `POPS`
+warning: load-endings.asm(50): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by `ENDSECTION`

test/asm/load-endings.out.bin

@@ -0,0 +1 @@
+BRRA
PQCS

test/asm/load-in-load.asm

@@ -1,5 +1,5 @@
 SECTION "outer", ROM0
-LOAD "inner", WRAM0
-LOAD "matryoshka", HRAM
-ENDL
-ENDL
+LOAD "inner1", WRAM0 ; starts "inner1"
+LOAD "inner2", HRAM ; ends "inner1", starts "inner2"
+ENDL ; ends "inner2"
+ENDL ; error

test/asm/load-in-load.err

@@ -1,5 +1,5 @@
-error: load-in-load.asm(3):
-    `LOAD` blocks cannot be nested
+warning: load-in-load.asm(3): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by `LOAD`
 error: load-in-load.asm(5):
     Found `ENDL` outside of a `LOAD` block
-error: Assembly aborted (2 errors)!
+error: Assembly aborted (1 error)!

test/asm/section-in-load.asm

@@ -1,4 +1,4 @@
-SECTION "outer", ROM0
+SECTION "outer1", ROM0
 LOAD "ram", WRAM0
-SECTION "inner", ROM0
+SECTION "outer2", ROM0
 ENDL

test/asm/section-in-load.err

@@ -1,2 +1,5 @@
-FATAL: section-in-load.asm(3):
-    Cannot change the section within a `LOAD` block
+warning: section-in-load.asm(3): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by `SECTION`
+error: section-in-load.asm(4):
+    Found `ENDL` outside of a `LOAD` block
+error: Assembly aborted (1 error)!

test/asm/unterminated-load.asm

@@ -0,0 +1,2 @@
+SECTION "rom", ROM0
+LOAD "ram", WRAM0

test/asm/unterminated-load.err

@@ -0,0 +1,2 @@
+warning: unterminated-load.asm(3): [-Wunterminated-load]
+    `LOAD` block without `ENDL` terminated by EOF