[Mesa-dev] [PATCH] glcpp: Rewrite line-continuation support to act globally.

From: Carl Worth <cworth at cworth.org>

Previously, we were only supporting line-continuation backslash characters
within lines of pre-processor directives, (as per the specification). With
OpenGL 4.2 and GLES3, line continuations are now supported anywhere within a
shader.

While changing this, also fix a bug where the preprocessor was ignoring
line continuation characters when a line ended in multiple backslash
characters.

The new code is also more efficient than the old. Previously, we would
perform a ralloc copy at each newline. We now perform copies only at each
occurrence of a line-continuation.

This commit fixes the line-continuation.vert test in piglit.
---

Compared to my previous (broken, infinite-looping) patch, this one adds back
the search_start variable from the original code. This prevents repeated
searching from the same starting location and infinitely finding the same
backslash.

As noted above, this commit is now tested and verified to fix the
line-continuation.vert test. Meanwhile, we also have a test which
is written specifically to fail if line continuation is performed
inside a comment, (comment-contination.frag). Presumably this was
inspired by Savage2. What shall we do this this test? Remove it?
Change it? Simply marking it as a test where the compilation is
expected to fail feels unattractive since there's no tight binding
between the desired behavior and how the test fails.


 src/glsl/glcpp/pp.c |  115 ++++++++++++++++++++++-----------------------------
 1 file changed, 49 insertions(+), 66 deletions(-)

diff --git a/src/glsl/glcpp/pp.c b/src/glsl/glcpp/pp.c
index 11b2941..fb4f77e 100644
--- a/src/glsl/glcpp/pp.c
+++ b/src/glsl/glcpp/pp.c
@@ -70,82 +70,65 @@ glcpp_warning (YYLTYPE *locp, glcpp_parser_t *parser, const char *fmt, ...)
 				     &parser->info_log_length, "\n");
 }
 
-/* Searches backwards for '^ *#' from a given starting point. */
-static int
-in_directive(const char *shader, const char *ptr)
-{
-	assert(ptr >= shader);
-
-	/* Search backwards for '#'. If we find a \n first, it doesn't count */
-	for (; ptr >= shader && *ptr != '#'; ptr--) {
-		if (*ptr == '\n')
-			return 0;
-	}
-	if (ptr >= shader) {
-		/* Found '#'...look for spaces preceded by a newline */
-		for (ptr--; ptr >= shader && isblank(*ptr); ptr--);
-		// FIXME: I don't think the '\n' case can happen
-		if (ptr < shader || *ptr == '\n')
-			return 1;
-	}
-	return 0;
-}
-
-/* Remove any line continuation characters in preprocessing directives.
- * However, ignore any in GLSL code, as "There is no line continuation
- * character" (1.30 page 9) in GLSL.
+/* Remove any line continuation characters in the shader, (whether in
+ * preprocessing directives or in GLSL code).
  */
 static char *
 remove_line_continuations(glcpp_parser_t *ctx, const char *shader)
 {
-	int in_continued_line = 0;
-	int extra_newlines = 0;
 	char *clean = ralloc_strdup(ctx, "");
-	const char *search_start = shader;
-	const char *newline;
-	while ((newline = strchr(search_start, '\n')) != NULL) {
-		const char *backslash = NULL;
-
-		/* # of characters preceding the newline. */
-		int n = newline - shader;
-
-		/* Find the preceding '\', if it exists */
-		if (n >= 1 && newline[-1] == '\\')
-			backslash = newline - 1;
-		else if (n >= 2 && newline[-1] == '\r' && newline[-2] == '\\')
-			backslash = newline - 2;
-
-		/* Double backslashes don't count (the backslash is escaped) */
-		if (backslash != NULL && backslash[-1] == '\\') {
-			backslash = NULL;
-		}
-
-		if (backslash != NULL) {
-			/* We found a line continuation, but do we care? */
-			if (!in_continued_line) {
-				if (in_directive(shader, backslash)) {
-					in_continued_line = 1;
-					extra_newlines = 0;
-				}
-			}
-			if (in_continued_line) {
-				/* Copy everything before the \ */
-				ralloc_strncat(&clean, shader, backslash - shader);
+	const char *backslash, *newline, *search_start;
+	int collapsed_newlines = 0;
+
+	search_start = shader;
+
+	while (true) {
+		backslash = strchr(search_start, '\\');
+
+		/* If we have previously collapsed any line-continuations,
+		 * then we want to insert additional newlines at the next
+		 * occurrence of a newline character to avoid changing any
+		 * line numbers.
+		 */
+		if (collapsed_newlines) {
+			newline = strchr(search_start, '\n');
+			if (newline &&
+			    (backslash == NULL || newline < backslash))
+			{
+				ralloc_strncat(&clean, shader,
+					       newline - shader + 1);
+				while (collapsed_newlines--)
+					ralloc_strcat(&clean, "\n");
 				shader = newline + 1;
-				extra_newlines++;
+				search_start = shader;
 			}
-		} else if (in_continued_line) {
-			/* Copy everything up to and including the \n */
-			ralloc_strncat(&clean, shader, newline - shader + 1);
-			shader = newline + 1;
-			/* Output extra newlines to make line numbers match */
-			for (; extra_newlines > 0; extra_newlines--)
-				ralloc_strcat(&clean, "\n");
-			in_continued_line = 0;
 		}
-		search_start = newline + 1;
+
+		search_start = backslash + 1;
+
+		if (backslash == NULL)
+			break;
+
+		/* At each line continuation, (backslash followed by a
+		 * newline), copy all preceding text to the output, then
+		 * advance the shader pointer to the character after the
+		 * newline.
+		 */
+		if (backslash[1] == '\n' ||
+		    (backslash[1] == '\r' && backslash[2] == '\n'))
+		{
+			collapsed_newlines++;
+			ralloc_strncat(&clean, shader, backslash - shader);
+			if (backslash[1] == '\n')
+				shader = backslash + 2;
+			else
+				shader = backslash + 3;
+			search_start = shader;
+		}
 	}
+
 	ralloc_strcat(&clean, shader);
+
 	return clean;
 }
 
-- 
1.7.10