improved "macro twice" error output and did some cleanup...

git-svn-id: https://svn.code.sf.net/p/acme-crossass/code-0/trunk@355 4df02467-bbd4-4a76-a152-e7ce94205b78

docs/Errors.txt

@@ -303,8 +303,8 @@ Loop var must be followed by either "in" keyword or comma.
 
 Macro already defined.
     Macros can only be defined once. If you define a macro twice, ACME
-    will help you find the definitions by giving a warning for the new
-    definition and then an error for the earlier definition.
+    will help you find the definitions by giving a warning for the
+    earlier definition and then an error for the new definition.
 
 Macro not defined (or wrong signature).
     You tried to call a macro that either wasn't defined yet (always

src/_riscos.c

@@ -90,13 +90,13 @@ void RISCOS_throwback(const char *message, int type)
 		regs.r[0] = 0;
 		regs.r[1] = 0;
 	//	regs.r[2] = (int) toplevel_source;
-		regs.r[2] = (int) input_now->location.filename;
+		regs.r[2] = (int) input_now->location.plat_filename;
 		_kernel_swi(XDDEUTILS_THROWBACKSEND, &regs, &regs);
 	}
 	// send throwback message
 	regs.r[0] = 1;
 	regs.r[1] = 0;
-	regs.r[2] = (int) input_now->location.filename;
+	regs.r[2] = (int) input_now->location.plat_filename;
 	regs.r[3] = input_now->location.line_number;
 	regs.r[4] = type;
 	regs.r[5] = (int) message;

src/acme.c

@@ -78,7 +78,7 @@ static const char	arg_vicelabels[]	= "VICE labels filename";
 
 
 // variables
-static const char	**toplevel_sources;
+static const char	**toplevel_sources_plat;	// source file names given on command line (platform-style)
 static int		toplevel_src_count	= 0;
 
 
@@ -323,11 +323,11 @@ static void perform_pass(void)
 	pass.warning_count = 0;
 	// Process toplevel files
 	for (ii = 0; ii < toplevel_src_count; ++ii) {
-		if ((fd = fopen(toplevel_sources[ii], FILE_READBINARY))) {
-			flow_parse_and_close_file(fd, toplevel_sources[ii]);
+		if ((fd = fopen(toplevel_sources_plat[ii], FILE_READBINARY))) {
+			flow_parse_and_close_platform_file(toplevel_sources_plat[ii], fd);
 		} else {
-			fprintf(stderr, "Error: Cannot open toplevel file \"%s\".\n", toplevel_sources[ii]);
-			if (toplevel_sources[ii][0] == '-')
+			fprintf(stderr, "Error: Cannot open toplevel file \"%s\".\n", toplevel_sources_plat[ii]);
+			if (toplevel_sources_plat[ii][0] == '-')
 				fprintf(stderr, "Options (starting with '-') must be given _before_ source files!\n");
  			++pass.error_count;
 		}
@@ -725,7 +725,7 @@ int main(int argc, const char *argv[])
 	// handle command line arguments
 	cliargs_handle_options(short_option, long_option);
 	// generate list of files to process
-	cliargs_get_rest(&toplevel_src_count, &toplevel_sources, "No top level sources given");
+	cliargs_get_rest(&toplevel_src_count, &toplevel_sources_plat, "No top level sources given");
 
 	// now that we have processed all cli switches, check a few values for
 	// valid range:

src/config.h

@@ -23,7 +23,8 @@ typedef unsigned long	uintval_t;	// just for logical shift right
 
 // struct to remember where macros were defined (FIXME - use for symbols as well!)
 struct location {
-	const char	*filename;
+	const char	*plat_filename;	// filename in platform style
+	// FIXME - add another field for filename in UNIX style? might be needed when fixing directory search order!
 	int		line_number;
 };
 

src/flow.c

@@ -267,17 +267,23 @@ void flow_do_while(struct do_while *loop)
 
 
 // parse a whole source code file
-void flow_parse_and_close_file(FILE *fd, const char *filename)
+// file name must be given in platform style, i.e.
+// "directory/basename.extension" on linux,
+// "directory.basename/extension" on RISC OS, etc.
+// and the pointer must remain valid forever!
+void flow_parse_and_close_platform_file(const char *plat_filename, FILE *fd)
 {
 	// be verbose
 	if (config.process_verbosity > 2)
-		printf("Parsing source file '%s'\n", filename);
+		printf("Parsing source file '%s'\n", plat_filename);
 	// set up new input
-	input_new_file(filename, fd);
+	input_new_platform_file(plat_filename, fd);
 	// parse block and check end reason
 	parse_until_eob_or_eof();
 	if (GotByte != CHAR_EOF)
 		Throw_error("Found '}' instead of end-of-file.");
 	// close sublevel src
+	// (this looks like we could just use "fd" as arg, but maybe the file
+	// has been replaced with a different one in the meantime...)
 	fclose(input_now->src.fd);
 }

src/flow.h

@@ -74,7 +74,11 @@ extern void flow_store_while_condition(struct condition *condition);
 extern void flow_do_while(struct do_while *loop);
 
 // parse a whole source code file
-extern void flow_parse_and_close_file(FILE *fd, const char *filename);
+// file name must be given in platform style, i.e.
+// "directory/basename.extension" on linux,
+// "directory.basename/extension" on RISC OS, etc.
+// and the pointer must remain valid forever!
+extern void flow_parse_and_close_platform_file(const char *plat_filename, FILE *fd);
 
 
 #endif

src/global.c

@@ -444,13 +444,13 @@ static void throw_msg(const char *message, const char *ansicolor, const char *ty
 
 	if (config.format_msvc) {
 		fprintf(config.msg_stream, "%s(%d) : %s%s%s (%s %s): %s\n",
-			input_now->location.filename, input_now->location.line_number,
+			input_now->location.plat_filename, input_now->location.line_number,
 			ansicolor, type, resetcolor,
 			section_now->type, section_now->title, message);
 	} else {
 		fprintf(config.msg_stream, "%s%s%s - File %s, line %d (%s %s): %s\n",
 			ansicolor, type, resetcolor,
-			input_now->location.filename, input_now->location.line_number,
+			input_now->location.plat_filename, input_now->location.line_number,
 			section_now->type, section_now->title, message);
 	}
 }

src/input.c

@@ -45,13 +45,18 @@ struct input	*input_now	= &outermost;	// current input structure
 // functions
 
 // let current input point to start of file
-void input_new_file(const char *filename, FILE *fd)
+// file name must be given in platform style, i.e.
+// "directory/basename.extension" on linux,
+// "directory.basename/extension" on RISC OS etc.
+// and the pointer must remain valid forever!
+// FIXME - there is only one caller, "flow_parse_and_close_platform_file()", so maybe merge?
+void input_new_platform_file(const char *plat_filename, FILE *fd)
 {
-	input_now->location.filename	= filename;
-	input_now->location.line_number	= 1;
-	input_now->source		= INPUTSRC_FILE;
-	input_now->state		= INPUTSTATE_SOF;
-	input_now->src.fd		= fd;
+	input_now->location.plat_filename	= plat_filename;
+	input_now->location.line_number		= 1;
+	input_now->source	= INPUTSRC_FILE;
+	input_now->state	= INPUTSTATE_SOF;
+	input_now->src.fd	= fd;
 }
 
 
@@ -67,7 +72,7 @@ static void report_srcchar(char new_char)
 
 	// if input has changed, insert explanation
 	if (input_now != report->last_input) {
-		fprintf(report->fd, "\n; ******** Source: %s\n", input_now->location.filename);
+		fprintf(report->fd, "\n; ******** Source: %s\n", input_now->location.plat_filename);	// actually, UNIX-style might be better than platform-style here...
 		report->last_input = input_now;
 		report->asc_used = 0;	// clear buffer
 		prev_char = '\0';
@@ -613,7 +618,7 @@ int input_read_and_lower_keyword(void)
 // shared ending when trying to read a file name.
 // The file name given in the assembler source code is converted from
 // UNIX style to platform style.
-// Returns nonzero on error. Filename in GlobalDynaBuf.
+// Returns nonzero on error. Filename in GlobalDynaBuf, including terminator.
 // Errors are handled and reported, but caller should call
 // input_skip_remainder() then.
 static int read_filename_shared_end(int prefix_size)
@@ -638,8 +643,10 @@ static int read_filename_shared_end(int prefix_size)
 }
 
 // try to read a file name for an input file.
-// library access by using <...> quoting is allowed. function will store info
-// about library usage at "uses_lib" ptr.
+// library access by using <...> quoting is allowed.
+// if library access is used, the library prefix will be added to the file name
+// and TRUE will be stored via the "uses_lib" ptr.
+// if library access is not used, FALSE will be stored via the "uses_lib" ptr.
 // The file name given in the assembler source code is converted from
 // UNIX style to platform style.
 // Returns nonzero on error. Filename in GlobalDynaBuf.
@@ -791,7 +798,7 @@ void includepaths_add(const char *path)
 }
 // open file for reading (trying list entries as prefixes)
 // "uses_lib" tells whether to access library or to make use of include paths
-// file name is expected in GlobalDynaBuf
+// file name is expected in GlobalDynaBuf, in platform style, and if wanted, with library prefix!
 FILE *includepaths_open_ro(boolean uses_lib)
 {
 	FILE		*stream;

src/input.h

@@ -60,7 +60,11 @@ extern struct input	*input_now;	// current input structure
 // Prototypes
 
 // let current input point to start of file
-extern void input_new_file(const char *filename, FILE *fd);
+// file name must be given in platform style, i.e.
+// "directory/basename.extension" on linux,
+// "directory.basename/extension" on RISC OS, etc.
+// and the pointer must remain valid forever!
+extern void input_new_platform_file(const char *plat_filename, FILE *fd);
 
 // get next byte from currently active byte source in shortened high-level
 // format. When inside quotes, use input_quoted_to_dynabuf() instead!
@@ -116,8 +120,10 @@ extern int input_read_keyword(void);
 extern int input_read_and_lower_keyword(void);
 
 // try to read a file name for an input file.
-// library access by using <...> quoting is allowed. function will store info
-// about library usage at "uses_lib" ptr.
+// library access by using <...> quoting is allowed.
+// if library access is used, the library prefix will be added to the file name
+// and TRUE will be stored via the "uses_lib" ptr.
+// if library access is not used, FALSE will be stored via the "uses_lib" ptr.
 // The file name given in the assembler source code is converted from
 // UNIX style to platform style.
 // Returns nonzero on error. Filename in GlobalDynaBuf.
@@ -152,7 +158,7 @@ extern void includepaths_add(const char *path);
 
 // open file for reading (trying list entries as prefixes)
 // "uses_lib" tells whether to access library or to make use of include paths
-// file name is expected in GlobalDynaBuf
+// file name is expected in GlobalDynaBuf, in platform style, and if wanted, with library prefix!
 extern FILE *includepaths_open_ro(boolean uses_lib);
 
 

src/macro.c

@@ -77,7 +77,7 @@ static scope_t get_scope_and_title(void)
 	// copy macro title to private dynabuf and add separator character
 	dynabuf_clear(user_macro_name);
 	dynabuf_add_string(user_macro_name, GLOBALDYNABUF_CURRENT);
-	dynabuf_append(user_macro_name, '\0');
+	dynabuf_append(user_macro_name, '\0');	// make sure terminator is part of buffer so get_copy() includes it!
 	dynabuf_clear(internal_name);
 	dynabuf_add_string(internal_name, GLOBALDYNABUF_CURRENT);
 	dynabuf_append(internal_name, ARG_SEPARATOR);
@@ -96,20 +96,6 @@ static int pipe_comma(void)
 	return result;
 }
 
-// Return malloc'd copy of string
-// (yes, we could also use strdup() from <string.h>, but then we still would
-// need to check for NULL etc.)
-static char *get_string_copy(const char *original)
-{
-	size_t	size;
-	char	*copy;
-
-	size = strlen(original) + 1;
-	copy = safe_malloc(size);
-	memcpy(copy, original, size);
-	return copy;
-}
-
 // This function is called from both macro definition and macro call.
 // Terminate macro name and copy from internal_name to GlobalDynaBuf
 // (because that's where tree_hard_scan() looks for the search string).
@@ -135,8 +121,6 @@ static void report_redefinition(struct rwnode *macro_node)
 	const char	*buffered_section_type;
 	char		*buffered_section_title;
 
-	// show warning with location of current definition
-	Throw_warning(exception_macro_twice);	// FIXME - throw as info?
 	// CAUTION, ugly kluge: fiddle with input_now and section_now
 	// data to generate helpful error messages
 	// FIXME - move this to a function so "symbol twice" error can also use it
@@ -148,12 +132,15 @@ static void report_redefinition(struct rwnode *macro_node)
 	input_now->location = original_macro->definition;
 	section_now->type = "earlier";
 	section_now->title = "definition";
-	// show error with location of earlier definition
-	Throw_error(exception_macro_twice);
+	// show warning with location of earlier definition
+	Throw_warning(exception_macro_twice);	// FIXME - throw as info?
 	// restore old data
 	input_now->location = buffered_location;
 	section_now->type = buffered_section_type;
 	section_now->title = buffered_section_title;
+
+	// show error with location of current definition
+	Throw_error(exception_macro_twice);
 }
 
 // This function is only called during the first pass, so there's no need to
@@ -210,9 +197,8 @@ void macro_parse_definition(void)	// Now GotByte = illegal char after "!macro"
 		report_redefinition(macro_node);
 	// Create new macro struct and set it up. Finally we'll read the body.
 	new_macro = safe_malloc(sizeof(*new_macro));
-	new_macro->definition.line_number = input_now->location.line_number;
-	new_macro->definition.filename = get_string_copy(input_now->location.filename);
-	new_macro->original_name = get_string_copy(user_macro_name->buffer);
+	new_macro->definition = input_now->location;
+	new_macro->original_name = dynabuf_get_copy(user_macro_name);
 	new_macro->parameter_list = formal_parameters;
 	new_macro->body = input_skip_or_store_block(TRUE);	// changes LineNumber
 	macro_node->body = new_macro;	// link macro struct to tree node

src/pseudoopcodes.c

@@ -368,9 +368,9 @@ static enum eos use_encoding_from_file(void)
 				*buffered_table;
 	const struct encoder	*buffered_encoder;
 
-	// read file name
+	// read file name and convert from UNIX style to platform style
 	if (input_read_input_filename(&uses_lib))
-		return SKIP_REMAINDER;	// missing or unterminated file name
+		return SKIP_REMAINDER;	// if missing or unterminated, give up
 
 	// read from file
 	stream = includepaths_open_ro(uses_lib);
@@ -547,9 +547,9 @@ static enum eos po_binary(void)
 	size.val.intval = -1;	// means "not given" => "until EOF"
 	skip.val.intval	= 0;
 
-	// if file name is missing, don't bother continuing
+	// read file name and convert from UNIX style to platform style
 	if (input_read_input_filename(&uses_lib))
-		return SKIP_REMAINDER;
+		return SKIP_REMAINDER;	// if missing or unterminated, give up
 
 	// try to open file
 	stream = includepaths_open_ro(uses_lib);
@@ -926,6 +926,7 @@ static enum eos po_source(void)	// now GotByte = illegal char
 {
 	boolean		uses_lib;
 	FILE		*stream;
+	const char	*eternal_plat_filename;
 	char		local_gotbyte;
 	struct input	new_input,
 			*outer_input;
@@ -934,30 +935,27 @@ static enum eos po_source(void)	// now GotByte = illegal char
 	// quit program if recursion too deep
 	if (--sanity.source_recursions_left < 0)
 		Throw_serious_error("Too deeply nested. Recursive \"!source\"?");
-	// read file name. quit function on error
+
+	// read file name and convert from UNIX style to platform style
 	if (input_read_input_filename(&uses_lib))
-		return SKIP_REMAINDER;
+		return SKIP_REMAINDER;	// if missing or unterminated, give up
 
 	// if file could be opened, parse it. otherwise, complain
 	stream = includepaths_open_ro(uses_lib);
 	if (stream) {
-// FIXME - just use safe_malloc and never free! this also saves us making a copy if defining macros down the road...
-#ifdef __GNUC__
-		char	filename[GlobalDynaBuf->size];	// GCC can do this
-#else
-		char	*filename	= safe_malloc(GlobalDynaBuf->size);	// VS can not
-#endif
-
-		strcpy(filename, GLOBALDYNABUF_CURRENT);
+		// GlobalDynaBuf contains either
+		//	library_prefix + platformstyle(source_argument) + '\0'
+		// or
+		//	platformstyle(source_argument) + '\0'
+		// it does _not_ contain any search path added using "-I", even if used!
+		// if this is a problem, fix includepaths_open_ro()!
+		eternal_plat_filename = dynabuf_get_copy(GlobalDynaBuf);
 		outer_input = input_now;	// remember old input
 		local_gotbyte = GotByte;	// CAUTION - ugly kluge
 		input_now = &new_input;	// activate new input
-		flow_parse_and_close_file(stream, filename);
+		flow_parse_and_close_platform_file(eternal_plat_filename, stream);
 		input_now = outer_input;	// restore previous input
 		GotByte = local_gotbyte;	// CAUTION - ugly kluge
-#ifndef __GNUC__
-		free(filename);	// GCC auto-frees
-#endif
 	}
 	// leave nesting level
 	++sanity.source_recursions_left;

src/version.h

@@ -9,7 +9,7 @@
 
 #define RELEASE		"0.97"		// update before release	FIXME
 #define CODENAME	"Zem"		// update before release
-#define CHANGE_DATE	"27 Feb"	// update before release	FIXME
+#define CHANGE_DATE	"28 Feb"	// update before release	FIXME
 #define CHANGE_YEAR	"2024"		// update before release
 //#define HOME_PAGE	"http://home.pages.de/~mac_bacon/smorbrod/acme/"
 #define HOME_PAGE	"http://sourceforge.net/p/acme-crossass/"	// FIXME

testing/errors/alreadydefined.inc

@@ -0,0 +1,7 @@
+; this gets included by symboltwice*.a and macrotwice*.a
+	!macro m @arg {
+		nop
+	}
+		nop
+label
+		nop

testing/errors/macrotwice1.a

@@ -0,0 +1,5 @@
+	*=$1000
+	!src "alreadydefined.inc"
+	!macro m @arg {
+		nop
+	}

testing/errors/macrotwice2.a

@@ -0,0 +1,5 @@
+	*=$1000
+	!macro m @arg {
+		nop
+	}
+	!src "alreadydefined.inc"

testing/errors/symboltwice1.a

@@ -0,0 +1,3 @@
+	*=$1000
+	!src "alreadydefined.inc"
+label

testing/errors/symboltwice2.a

@@ -0,0 +1,3 @@
+	*=$1000
+label
+	!src "alreadydefined.inc"