merge check output code into master

Author: andrew-taylor

README.md

@@ -140,8 +140,8 @@ valgrind also usually detect this type of error, e.g.:
 * Installation on Linux and Windows Subsystem for Linux
 
 	```bash
-	curl -L https://github.com/COMP1511UNSW/dcc/releases/download/2.2/dcc_2.2_all.deb -o /tmp/dcc_2.2_all.deb
-	sudo apt install /tmp/dcc_2.2_all.deb
+	curl -L https://github.com/COMP1511UNSW/dcc/releases/download/2.5/dcc_2.5_all.deb -o /tmp/dcc_2.5_all.deb
+	sudo apt install /tmp/dcc_2.5_all.deb
     # on WSL (not Linux) this might be necessary to run programs
 	sudo bash -c "echo 0 > /proc/sys/kernel/yama/ptrace_scope;echo 1 >/proc/sys/vm/overcommit_memory"
 	```
@@ -152,7 +152,7 @@ valgrind also usually detect this type of error, e.g.:
 	Install gdb - see https://sourceware.org/gdb/wiki/PermissionsDarwin
 
     ```bash
-	sudo curl -L https://github.com/COMP1511UNSW/dcc/releases/download/2.2/dcc -o /usr/local/bin/dcc
+	sudo curl -L https://github.com/COMP1511UNSW/dcc/releases/download/2.5/dcc -o /usr/local/bin/dcc
 	sudo chmod o+rx  /usr/local/bin/dcc
     ```
 

__main__.py

@@ -33,7 +33,7 @@ def main():
 	signal.signal(signal.SIGINT, handler)
 	debug = int(os.environ.get('DCC_DEBUG', '0'))
 	colorize_output = sys.stderr.isatty() or os.environ.get('DCC_COLORIZE_OUTPUT', False)
-	if debug: print(sys.argv, 'DCC_RUN_INSIDE_GDB="%s" DCC_PID="%s"' % (os.environ.get('DCC_RUN_INSIDE_GDB', ''), os.environ.get('DCC_PID', '')), file=sys.stderr)
+	if debug > 1: print(sys.argv, 'DCC_RUN_INSIDE_GDB="%s" DCC_PID="%s"' % (os.environ.get('DCC_RUN_INSIDE_GDB', ''), os.environ.get('DCC_PID', '')), file=sys.stderr)
 	if not sys.argv[1:] and 'DCC_RUN_INSIDE_GDB' in os.environ:
 		drive_gdb()
 	elif not sys.argv[1:] and 'DCC_PID' in os.environ:

compile.py

@@ -79,12 +79,14 @@ def get_short_explanation(self, message, colorize_output):
 		if colorize_output:
 			color = colors.color
 		else:
-			color = lambda text, color_name: text
+			color = lambda text, *args, **kwargs: text
 
 		parameters = dict((name, getattr(message, name)) for name in dir(message) if not name.startswith('__'))
 		parameters['match'] = match
 		parameters['color'] = color
-		parameters['emphasize'] = lambda text: color(text, 'red')
+		parameters['emphasize'] = lambda text: color(text, style='bold')
+		parameters['danger'] = lambda text: color(text, color='red', style='bold')
+		parameters['info'] = lambda text: color(text, 'cyan', style='bold')
 		return eval('f"' + self.explanation.replace('\n', '\\n') +'"',  globals(), parameters)
 
 explanations = [
@@ -450,14 +452,138 @@ def get_short_explanation(self, message, colorize_output):
 
 		regex = r"indirection requires pointer operand \('(.*)' invalid\)",
 
-		explanation = """ you are trying to use '{emphasize(underlined_word)}' as a pointer.
+		explanation = """You are trying to use '{emphasize(underlined_word)}' as a pointer.
   You can not do this because '{emphasize(underlined_word)}' is of type {emphasize(match.group(1))}.
 """,
 
 		reproduce = """
 int main(int argc, char *argv[]) {
 	return *argc;
 }
+""",
+	),
+	
+	Explanation(
+		label = 'duplicated-cond',
+		
+		regex = r"duplicated .*\bif\b.* condition",
+		
+		explanation = """You have repeated the same condition in a chain of if statements.
+Only the first if statement using the condition can be executed.
+The others can never be executed.
+
+""",
+		
+		reproduce = """
+int main(int argc, char *argv[]) {
+	if (argc == 1)
+		return 42;
+	else if (argc == 1)
+    	return 43;
+ 	else
+		return 44;
+}
+""",
+	),
+	
+	Explanation(
+		label = 'duplicated-branches',
+		
+		regex = r"condition has identical branches",
+		
+		explanation = """Your if statement has identical then and else parts.
+It is pointless to have an if statement which executes the same code
+when its condition is true and also when its condition is false.
+
+""",
+		
+		reproduce = """
+int main(int argc, char *argv[]) {
+	if (argc == 1)
+		return 42;
+	else 
+    	return 42;
+}
+""",
+	),
+	
+	Explanation(
+		label = 'logical-or-always-true',
+		
+		regex = r"logical .?\bor\b.* is always true",
+		
+		explanation = """Your '{emphasize('||')}' expression is always true, no matter what value variables have.
+Perhaps you meant to use '{emphasize('&&')}' ?
+
+""",
+		
+		reproduce = """
+int main(int argc, char *argv[]) {
+	if (argc > 1 || argc < 3)
+		return 42;
+	else 
+    	return 43;
+}
+""",
+	),
+	
+	Explanation(
+		label = 'logical-and-always-false',
+		
+		regex = r"logical .?\band\b.* is always false",
+		
+		explanation = """Your '{emphasize('&&')}' expression is always false, no matter what value variables have.
+Perhaps you meant to use '{emphasize('||')}' ?
+
+""",
+		
+		reproduce = """
+int main(int argc, char *argv[]) {
+	if (argc > 1 && argc < 1)
+		return 42;
+	else 
+    	return 43;
+}
+""",
+	),
+	
+	Explanation(
+		label = 'logical-equal-expressions',
+		
+		regex = r"logical .?((and|or)).? of equal expressions",
+		
+		explanation = """Your have used '{emphasize(highlighted_word)}' with same lefthand and righthand operands.
+If this what you meant, it can be simplified: {emphasize('x ' + highlighted_word + ' x')} can be replaced with just {emphasize('x')}.
+
+""",
+		
+		reproduce = """
+int main(int argc, char *argv[]) {
+	if (argc > 1 ||argc > 1)
+		return 42;
+	else 
+    	return 43;
+}
+""",
+	),
+	
+	Explanation(
+		label = 'shadow-local-variable',
+		
+		regex = r"declaration shadows a local variable",
+		
+		explanation = """Your already have a variable named '{emphasize(highlighted_word)}'.
+It is confusing to have a second overlapping declaration of the same variable name.
+
+""",
+		
+		reproduce = """
+int main(int argc, char *argv[]) {
+	{
+		int argc = 42;
+		return argc;
+	}
+}
 """,
 	),
 ]

compiler_explanations.py

@@ -11,6 +11,7 @@ static void __dcc_check_close(int fd) {
 }
 
 static void __dcc_check_output_exit(void) {
+	debug_printf(3, "__dcc_cleanup_before_exit\n");
 	fflush(stdout);
 }
 
@@ -26,14 +27,14 @@ static void disable_check_output(void) {
 //lins longer than this will produce an error if output checking enabled
 #define ACTUAL_LINE_MAX 65536
 
+static unsigned char *expected_stdout;
+
 static int ignore_case;
 static int ignore_empty_lines;
 static int ignore_trailing_white_space;
 static int ignore_characters[N_ASCII];
 static int max_stdout_bytes;
 
-static unsigned char *expected_stdout;
-
 static int getenv_boolean(const char *name, int default_value);
 
 static int init_check_output(void) {
@@ -117,8 +118,9 @@ static void __dcc_check_close(int fd) {
 }
 
 static void __dcc_check_output_exit(void) {
-	fflush(stdout);
+	debug_printf(3, "__dcc_cleanup_before_exit\n");
 	if (expected_stdout) {
+		fflush(stdout);
 		__dcc_check_all_output_seen_at_exit();
 	}
 }

dcc_check_output.c

@@ -162,6 +162,7 @@ int __wrap_main(int argc, char *argv[], char *envp[]) {
 	return 1; // not reached
 }
 
+
 static void __dcc_main_sanitizer1(int argc, char *argv[], char *envp[]) {
 	debug_printf(2, "main sanitizer1\n");
 	close(to_sanitizer2_pipe[0]);

dcc_main.c

@@ -374,7 +374,12 @@ def parse_gdb_stack_frame(line):
 	debug_print(3, 'parse_gdb_stack_frame', m != None, line)
 	if m:
 		filename = m.group('filename')
-		if filename.startswith("/usr/") or filename.startswith("../sysdeps/") or filename.endswith("libioP.h"): 
+		if (
+			filename.startswith("/usr/") or
+			filename.startswith("../sysdeps/") or
+			filename.endswith("libioP.h") or 
+			filename.endswith("iofclose.c") 
+		   ): 
 			m = None
 	if m:
 		return Location(m.group('filename'), m.group('line_number'), function=m.group('function'), params=m.group('params'), frame_number=m.group('frame_number'))

drive_gdb.py

@@ -12,11 +12,11 @@ export DCC_EXPECTED_STDOUT="  ___
 
 for bird in "$bird_directory"/bird-*.c
 do
-	basename "$bird"
+	basename "$bird" 1>&2
 	$dcc "$bird"
-	./a.out || continue
-	echo "Test $bird failed" 
+	./a.out >/dev/null || continue
+	echo "Test $bird failed" 1>&2
 	exit 1
 done
-echo "All Tests Correct"
+echo "All Tests Correct" 1>&2
 exit 0

tests/check_output/check_birds.sh

@@ -19,12 +19,12 @@ function run_test {
 	test_number=$((test_number + 1))
 
 	(
-		echo "*** Test $test_number - $label"
-		echo "*** Test Environment $environment_variables"
-		echo "expected output: $DCC_EXPECTED_STDOUT"
-		echo "actual output  : $actual_output"
+		echo "*** Test $test_number - $label" 1>&2
+		echo "*** Test Environment $environment_variables" 1>&2
+		echo "expected output: $DCC_EXPECTED_STDOUT" 1>&2
+		echo "actual output  : $actual_output" 1>&2
 
-		eval "$environment_variables"' ./a.out "$actual_output"'
+		eval "$environment_variables"' ./a.out "$actual_output" >/dev/null'
 		exit_status=$?
 
 		case "$label" in
@@ -34,7 +34,7 @@ function run_test {
 
 		if (((exit_status != 0)  != $expected_result_status))
 		then
-			echo "*** Test failed ***"
+			echo "*** Test failed ***" 1>&2
 			exit 1
 		fi
 	) || exit 1
@@ -68,5 +68,5 @@ run_test compare-only-characters-3-fail "DCC_COMPARE_ONLY_CHARACTERS=hel"
 run_test compare-only-characters-4      "DCC_IGNORE_CASE=1 DCC_COMPARE_ONLY_CHARACTERS=hel" $'Hell\n'
 
 
-echo All Tests Correct
+echo All Tests Correct 1>&2
 

tests/check_output/simple_check_output.sh

@@ -9,7 +9,7 @@ done
 tests_dir=$(dirname $(readlink -f $0))
 cd "$tests_dir"
 
-trap 'rm -fr tmp* a.out;exit' EXIT INT TERM
+trap 'exit_status=$?;rm -fr tmp* a.out;exit $exit_status' EXIT INT TERM
 
 # some values reported in errors are not determinate (e.g. variable addresses)
 # and will vary between execution and definitely between platforms