r369986 - [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

[Richard Smith via cfe-commits]

On Mon, 26 Aug 2019 at 16:17, Alexandre Ganea via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: aganea
> Date: Mon Aug 26 16:19:21 2019
> New Revision: 369986
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369986&view=rev
> Log:
> [clang-scan-deps] Minimizer: Correctly handle multi-line content with
> CR+LF line endings
>
> Previously, an #error directive with quoted, multi-line content, along
> with CR+LF line endings wasn't handled correctly.
>
> Differential Revision: https://reviews.llvm.org/D66556
>
> Added:
>
> cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
> Modified:
>     cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
>
> Modified: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp?rev=369986&r1=369985&r2=369986&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp (original)
> +++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp Mon Aug 26
> 16:19:21 2019
> @@ -196,15 +196,29 @@ static void skipString(const char *&Firs
>      ++First; // Finish off the string.
>  }
>
> -static void skipNewline(const char *&First, const char *End) {
> -  assert(isVerticalWhitespace(*First));
> -  ++First;
> +// Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2
> (\r\n)
> +static unsigned isEOL(const char *First, const char *const End) {
>    if (First == End)
> -    return;
> +    return 0;
> +  if (End - First > 1 && isVerticalWhitespace(First[0]) &&
> +      isVerticalWhitespace(First[1]) && First[0] != First[1])
> +    return 2;
> +  return !!isVerticalWhitespace(First[0]);
> +}
> +
> +// Returns the length of the skipped newline
> +static unsigned skipNewline(const char *&First, const char *End) {
> +  if (First == End)
> +    return 0;
> +  assert(isVerticalWhitespace(*First));
> +  unsigned Len = isEOL(First, End);
> +  assert(Len && "expected newline");
> +  First += Len;
> +  return Len;
> +}
>
> -  // Check for "\n\r" and "\r\n".
> -  if (LLVM_UNLIKELY(isVerticalWhitespace(*First) && First[-1] !=
> First[0]))
> -    ++First;
> +static bool wasLineContinuation(const char *First, unsigned EOLLen) {
> +  return *(First - (int)EOLLen - 1) == '\\';
>  }
>
>  static void skipToNewlineRaw(const char *&First, const char *const End) {
> @@ -212,17 +226,21 @@ static void skipToNewlineRaw(const char
>      if (First == End)
>        return;
>
> -    if (isVerticalWhitespace(*First))
> +    unsigned Len = isEOL(First, End);
> +    if (Len)
>        return;
>
> -    while (!isVerticalWhitespace(*First))
> +    do {
>        if (++First == End)
>          return;
> +      Len = isEOL(First, End);
> +    } while (!Len);
>
>      if (First[-1] != '\\')
>        return;
>
> -    ++First; // Keep going...
> +    First += Len;
> +    // Keep skipping lines...
>    }
>  }
>
> @@ -277,7 +295,7 @@ static bool isQuoteCppDigitSeparator(con
>  }
>
>  static void skipLine(const char *&First, const char *const End) {
> -  do {
> +  for (;;) {
>      assert(First <= End);
>      if (First == End)
>        return;
> @@ -322,9 +340,10 @@ static void skipLine(const char *&First,
>        return;
>
>      // Skip over the newline.
> -    assert(isVerticalWhitespace(*First));
> -    skipNewline(First, End);
> -  } while (First[-2] == '\\'); // Continue past line-continuations.
> +    unsigned Len = skipNewline(First, End);
> +    if (!wasLineContinuation(First, Len)) // Continue past
> line-continuations.
> +      break;
> +  }
>  }
>
>  static void skipDirective(StringRef Name, const char *&First,
> @@ -379,6 +398,8 @@ void Minimizer::printToNewline(const cha
>      // Print out the string.
>      if (Last == End || Last == First || Last[-1] != '\\') {
>        append(First, reverseOverSpaces(First, Last));
> +      First = Last;
> +      skipNewline(First, End);
>        return;
>      }
>
>
> Added:
> cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c?rev=369986&view=auto
>
> ==============================================================================
> ---
> cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
> (added)
> +++
> cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
> Mon Aug 26 16:19:21 2019
> @@ -0,0 +1,16 @@
> +// Test CF+LF are properly handled along with quoted, multi-line #error
> +// RUN: cat %s | unix2dos | %clang_cc1 -DOTHER
> -print-dependency-directives-minimized-source 2>&1 | FileCheck %s
>

You can't rely on 'unix2dos' existing across all our supported build
environments; this is causing a test failure for me.

I'm reverting for now. Maybe you could just check in a file with CRLF line
endings for this test (marked as binary rather than text so that clients
don't "fix" it)?


> +
> +#ifndef TEST
> +#error "message \
> +   more message \
> +   even more"
> +#endif
> +
> +#ifdef OTHER
> +#include <string>
> +#endif
> +
> +// CHECK:      #ifdef OTHER
> +// CHECK-NEXT: #include <string>
> +// CHECK-NEXT: #endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190826/7a5e1522/attachment-0001.html>