Fix line continuations for preprocessor directives

Line continuations in preprocessor directives are now handled correctly
such that assignments like the following are possible:

VAR=foo\
!IF 1
bar\
!EL\
SE
baz\
!ENDIF
zap

which evaluates to

VAR=foo bar zap

Task-number: QTCREATORBUG-8621
Task-number: QTCREATORBUG-18001
Change-Id: I3f86c6e3b13cbc04451fcfb267c5c43946b8e85c
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>

5 files changed, 178 insertions, 132 deletions

diff --git a/src/jomlib/makefilelinereader.cpp b/src/jomlib/makefilelinereader.cpp
index b116b67..60a1cbf 100644
--- a/src/jomlib/makefilelinereader.cpp
+++ b/src/jomlib/makefilelinereader.cpp
@@ -88,11 +88,11 @@ void MakefileLineReader::growLineBuffer(size_t nGrow)
  *    - combines multi-lines (lines with \ at the end) into a single long line
  *    - handles the ^ escape character for \ and \n at the end
  */
-QString MakefileLineReader::readLine(bool bInlineFileMode)
+MakefileLine MakefileLineReader::readLine(bool bInlineFileMode)
 {
     if (bInlineFileMode) {
         m_nLineNumber++;
-        return QString::fromLatin1(m_file.readLine());
+        return MakefileLine{ QString::fromLatin1(m_file.readLine()) };
     }
 
     return (this->*m_readLineImpl)();
@@ -101,83 +101,56 @@ QString MakefileLineReader::readLine(bool bInlineFileMode)
 /**
  * readLine implementation optimized for 8 bit files.
  */
-QString MakefileLineReader::readLine_impl_local8bit()
+MakefileLine MakefileLineReader::readLine_impl_local8bit()
 {
-    QString line;
-    bool multiLineAppendix = false;
-    bool endOfLineReached = false;
     size_t bytesRead;
     do {
-        do {
-            m_nLineNumber++;
-            const qint64 n = m_file.readLine(m_lineBuffer, m_nLineBufferSize - 1);
-            if (n <= 0)
-                return QString();
-
-            bytesRead = n;
-            while (m_lineBuffer[bytesRead - 1] != '\n') {
-                if (m_file.atEnd()) {
-                    // The file didn't end with a newline.
-                    // Code below relies on having a trailing newline.
-                    // We're imitating it by increasing the string length.
-                    if (bytesRead >= (m_nLineBufferSize - 2))
-                        growLineBuffer(1);
-                    ++bytesRead;
-                    break;
-                }
-
-                growLineBuffer(m_nLineBufferGrowSize);
-                int moreBytesRead = m_file.readLine(m_lineBuffer + bytesRead, m_nLineBufferSize - 1 - bytesRead);
-                if (moreBytesRead <= 0)
-                    break;
-                bytesRead += moreBytesRead;
+        m_nLineNumber++;
+        const qint64 n = m_file.readLine(m_lineBuffer, m_nLineBufferSize - 1);
+        if (n <= 0)
+            return {};
+
+        bytesRead = n;
+        while (m_lineBuffer[bytesRead - 1] != '\n') {
+            if (m_file.atEnd()) {
+                // The file didn't end with a newline.
+                // Code below relies on having a trailing newline.
+                // We're imitating it by increasing the string length.
+                if (bytesRead >= (m_nLineBufferSize - 2))
+                    growLineBuffer(1);
+                ++bytesRead;
+                break;
             }
 
-        } while (m_lineBuffer[0] == '#');
-        m_lineBuffer[bytesRead] = '\0';
-
-        char* buf = m_lineBuffer;
-        int bufLength = static_cast<int>(bytesRead);
-        if (multiLineAppendix) {
-            // skip leading whitespace characters
-            while (*buf && (*buf == ' ' || *buf == '\t'))
-                buf++;
-            if (buf != m_lineBuffer) {
-                buf--;          // make sure, we keep one whitespace
-                *buf = ' ';     // convert possible tab to space
-                bufLength -= buf - m_lineBuffer;
-            }
+            growLineBuffer(m_nLineBufferGrowSize);
+            int moreBytesRead = m_file.readLine(m_lineBuffer + bytesRead, m_nLineBufferSize - 1 - bytesRead);
+            if (moreBytesRead <= 0)
+                break;
+            bytesRead += moreBytesRead;
         }
 
-        if (bufLength >= 2 && buf[bufLength - 2] == '\\') {
-            if (bufLength >= 3 && buf[bufLength - 3] == '^') {
-                buf[bufLength - 3] = '\\';      // replace "^\\\n" -> "\\\\\n"
-                bufLength -= 2;                 // remove "\\\n"
-                line.append(QString::fromLatin1(buf, bufLength));
-                endOfLineReached = true;
-            } else {
-                bufLength -= 2; // remove "\\\n"
-                line.append(QString::fromLatin1(buf, bufLength));
-                multiLineAppendix = true;
-            }
-        } else if (bufLength >= 2 && buf[bufLength - 2] == '^') {
-            bufLength--;
-            buf[bufLength-1] = '\n';
-            line.append(QString::fromLatin1(buf, bufLength));
-            multiLineAppendix = true;
+    } while (m_lineBuffer[0] == '#');
+    m_lineBuffer[bytesRead] = '\0';
+
+    MakefileLine line;
+    char* buf = m_lineBuffer;
+    int bufLength = static_cast<int>(bytesRead);
+    if (bufLength >= 2 && buf[bufLength - 2] == '\\') {
+        if (bufLength >= 3 && buf[bufLength - 3] == '^') {
+            buf[bufLength - 3] = '\\';      // replace "^\\\n" -> "\\\\\n"
+            bufLength -= 2;                 // remove "\\\n"
         } else {
-            bufLength--;    // remove trailing \n
-            line.append(QString::fromLatin1(buf, bufLength));
-            endOfLineReached = true;
+            bufLength -= 2; // remove "\\\n"
+            line.continuation = LineContinuationType::Backslash;
         }
-    } while (!endOfLineReached);
-
-    // trim whitespace from the right
-    int idx = line.length() - 1;
-    while (idx > 0 && isSpaceOrTab(line.at(idx)))
-        --idx;
-    line.truncate(idx+1);
+    } else if (bufLength >= 2 && buf[bufLength - 2] == '^') {
+        bufLength -= 2;
+        line.continuation = LineContinuationType::Caret;
+    } else {
+        bufLength--;    // remove trailing \n
+    }
 
+    line.content = QString::fromLatin1(buf, bufLength);
     return line;
 }
 
@@ -185,41 +158,29 @@ QString MakefileLineReader::readLine_impl_local8bit()
  * readLine implementation for unicode files.
  * Much slower than the 8 bit version.
  */
-QString MakefileLineReader::readLine_impl_unicode()
+MakefileLine MakefileLineReader::readLine_impl_unicode()
 {
-    QString line;
-    bool endOfLineReached = false;
-    bool multilineAppendix = false;
+    MakefileLine line;
+    QString str;
     do {
         m_nLineNumber++;
-        QString str = m_textStream.readLine();
-        if (str.isNull())
-            break;
-
-        if (str.startsWith(QLatin1Char('#')))
-            continue;
-
-        if (multilineAppendix && (str.startsWith(QLatin1Char(' ')) || str.startsWith(QLatin1Char('\t')))) {
-            str = str.trimmed();
-            str.prepend(QLatin1Char(' '));
-        }
-
-        if (str.endsWith(QLatin1String("^\\"))) {
-            str.remove(str.length() - 2, 1);
-            endOfLineReached = true;
-        } else if (str.endsWith(QLatin1Char('\\'))) {
-            str.chop(1);
-            multilineAppendix = true;
-        } else if (str.endsWith(QLatin1Char('^'))) {
-            str.chop(1);
-            str.append(QLatin1Char('\n'));
-            multilineAppendix = true;
-        } else {
-            endOfLineReached = true;
-        }
+        str = m_textStream.readLine();
+    } while (str.startsWith(QLatin1Char('#')));
+
+    if (str.isNull())
+        return line;
+
+    if (str.endsWith(QLatin1String("^\\"))) {
+        str.remove(str.length() - 2, 1);
+    } else if (str.endsWith(QLatin1Char('\\'))) {
+        str.chop(1);
+        line.continuation = LineContinuationType::Backslash;
+    } else if (str.endsWith(QLatin1Char('^'))) {
+        str.chop(1);
+        line.continuation = LineContinuationType::Caret;
+    }
 
-        line.append(str);
-    } while (!endOfLineReached);
+    line.content = str;
     return line;
 }
 
diff --git a/src/jomlib/makefilelinereader.h b/src/jomlib/makefilelinereader.h
index a7edc7e..fdf3394 100644
--- a/src/jomlib/makefilelinereader.h
+++ b/src/jomlib/makefilelinereader.h
@@ -31,6 +31,24 @@
 
 namespace NMakeFile {
 
+enum class LineContinuationType
+{
+    None,
+    Backslash,
+    Caret
+};
+
+struct MakefileLine
+{
+    QString content;
+    LineContinuationType continuation = LineContinuationType::None;
+};
+
+inline bool isComplete(const MakefileLine &line)
+{
+    return line.continuation == LineContinuationType::None;
+}
+
 class MakefileLineReader {
 public:
     MakefileLineReader(const QString& filename);
@@ -38,17 +56,17 @@ public:
 
     bool open();
     void close();
-    QString readLine(bool bInlineFileMode);
+    MakefileLine readLine(bool bInlineFileMode);
     QString fileName() const { return m_file.fileName(); }
     uint lineNumber() const { return m_nLineNumber; }
 
 private:
     void growLineBuffer(size_t nGrow);
 
-    typedef QString (MakefileLineReader::*ReadLineImpl)();
+    typedef MakefileLine (MakefileLineReader::*ReadLineImpl)();
     ReadLineImpl m_readLineImpl;
-    QString readLine_impl_local8bit();
-    QString readLine_impl_unicode();
+    MakefileLine readLine_impl_local8bit();
+    MakefileLine readLine_impl_unicode();
 
 private:
     QFile m_file;
diff --git a/src/jomlib/preprocessor.cpp b/src/jomlib/preprocessor.cpp
index 5f78b53..97c28c2 100644
--- a/src/jomlib/preprocessor.cpp
+++ b/src/jomlib/preprocessor.cpp
@@ -27,7 +27,6 @@
 #include "ppexprparser.h"
 #include "macrotable.h"
 #include "exception.h"
-#include "makefilelinereader.h"
 #include "helperfunctions.h"
 #include "fastfileinfo.h"
 
@@ -96,20 +95,31 @@ bool Preprocessor::internalOpenFile(QString fileName)
 
 QString Preprocessor::readLine()
 {
-    QString line;
+    MakefileLine line;
     for (;;) {
-        basicReadLine(line);
-        if (!m_bInlineFileMode && parseMacro(line))
+        MakefileLine next = basicReadLine();
+        if (next.content.startsWith(QLatin1Char('!'))) {
+            completePreprocessingDirectiveLine(next);
+            parsePreprocessingDirective(next.content);
             continue;
-        if (parsePreprocessingDirective(line))
+        }
+        if (isComplete(line))
+            line = std::move(next);
+        else
+            joinLines(line, next);
+        if (!isComplete(line))
+            continue;
+        if (!m_bInlineFileMode && parseMacro(line.content))
+            continue;
+        if (parsePreprocessingDirective(line.content))
             continue;
         break;
     }
 
-    if (line.isNull() && conditionalDepth())
+    if (line.content.isNull() && conditionalDepth())
         error(QLatin1Literal("Missing !ENDIF directive."));
 
-    return line;
+    return line.content;
 }
 
 uint Preprocessor::lineNumber() const
@@ -126,26 +136,72 @@ QString Preprocessor::currentFileName() const
     return m_fileStack.top().reader->fileName();
 }
 
-void Preprocessor::basicReadLine(QString& line)
+MakefileLine Preprocessor::basicReadLine()
 {
-    if (!m_linesPutBack.isEmpty()) {
-        line = m_linesPutBack.takeFirst();
-        return;
-    }
+    if (!m_linesPutBack.isEmpty())
+        return { m_linesPutBack.takeFirst() };
 
-    if (m_fileStack.isEmpty()) {
-        line = QString();
-        return;
-    }
+    if (m_fileStack.isEmpty())
+        return {};
 
-    line = m_fileStack.top().reader->readLine(m_bInlineFileMode);
-    while (line.isNull()) {
+    MakefileLine line = m_fileStack.top().reader->readLine(m_bInlineFileMode);
+    while (line.content.isNull()) {
         delete m_fileStack.top().reader;
         m_fileStack.pop();
         if (m_fileStack.isEmpty())
-            return;
+            break;
         line = m_fileStack.top().reader->readLine(m_bInlineFileMode);
     }
+    return line;
+}
+
+static QString leftTrimmed(const QString &str)
+{
+    if (str.isEmpty())
+        return str;
+    int n = 0;
+    for (; n < str.length() && str.at(n).isSpace(); ++n);
+    return str.mid(n);
+}
+
+void Preprocessor::joinLines(MakefileLine &line, const MakefileLine &next)
+{
+    if (line.continuation == LineContinuationType::Backslash)
+        line.content += QLatin1Char(' ');
+    else if (line.continuation == LineContinuationType::Caret)
+        line.content += QLatin1Char('\n');
+    line.content += leftTrimmed(next.content);
+    line.continuation = next.continuation;
+}
+
+void Preprocessor::joinPreprocessingDirectiveLines(MakefileLine &line, const MakefileLine &next)
+{
+    if (line.continuation == LineContinuationType::Caret)
+        line.content += QLatin1Char('^');
+    line.content += next.content;
+    line.continuation = next.continuation;
+}
+
+void Preprocessor::completeLineImpl(MakefileLine &line, JoinFunc join)
+{
+    while (!isComplete(line)) {
+        MakefileLine next = basicReadLine();
+        if (next.content.isNull()) {
+            line.continuation = LineContinuationType::None;
+            break;
+        }
+        join(line, next);
+    }
+}
+
+void Preprocessor::completeLine(MakefileLine &line)
+{
+    completeLineImpl(line, &joinLines);
+}
+
+void Preprocessor::completePreprocessingDirectiveLine(MakefileLine &line)
+{
+    completeLineImpl(line, &joinPreprocessingDirectiveLines);
 }
 
 bool Preprocessor::parseMacro(const QString& line)
@@ -258,6 +314,8 @@ bool Preprocessor::parsePreprocessingDirective(const QString& line)
         exitConditional();
     } else if (directive == QLatin1String("UNDEF")) {
         m_macroTable->undefineMacro(value);
+    } else {
+        error(QString(QStringLiteral("Unknown preprocessor directive !%1")).arg(directive));
     }
 
     return true;
@@ -345,16 +403,20 @@ bool Preprocessor::isPreprocessingDirective(const QString& line, QString& direct
 void Preprocessor::skipUntilNextMatchingConditional()
 {
     uint depth = 0;
-    QString line, directive, value;
+    MakefileLine line;
+    QString directive, value;
 
     enum DirectiveToken { TOK_IF, TOK_ENDIF, TOK_ELSE, TOK_UNINTERESTING };
     DirectiveToken token;
     do {
-        basicReadLine(line);
-        if (line.isNull())
+        line = basicReadLine();
+        if (line.content.isNull())
             return;
 
-        QString expandedLine = m_macroTable->expandMacros(line);
+        if (line.content.startsWith(QLatin1Char('!')))
+            completePreprocessingDirectiveLine(line);
+
+        QString expandedLine = m_macroTable->expandMacros(line.content);
         if (!isPreprocessingDirective(expandedLine, directive, value))
             continue;
 
@@ -386,7 +448,7 @@ void Preprocessor::skipUntilNextMatchingConditional()
         else if (token == TOK_IF)
             ++depth;
 
-    } while (!line.isNull());
+    } while (!line.content.isNull());
 }
 
 void Preprocessor::enterConditional(bool followElseBranch)
diff --git a/src/jomlib/preprocessor.h b/src/jomlib/preprocessor.h
index 02a7d69..b268c72 100644
--- a/src/jomlib/preprocessor.h
+++ b/src/jomlib/preprocessor.h
@@ -26,6 +26,8 @@
 #ifndef PREPROCESSOR_H
 #define PREPROCESSOR_H
 
+#include "makefilelinereader.h"
+
 #include <QRegExp>
 #include <QStack>
 #include <QStringList>
@@ -57,7 +59,13 @@ public:
 
 private:
     bool internalOpenFile(QString fileName);
-    void basicReadLine(QString& line);
+    MakefileLine basicReadLine();
+    typedef void (*JoinFunc)(MakefileLine &, const MakefileLine &);
+    static void joinLines(MakefileLine &line, const MakefileLine &next);
+    static void joinPreprocessingDirectiveLines(MakefileLine &line, const MakefileLine &next);
+    void completeLineImpl(MakefileLine &line, JoinFunc joinFunc);
+    void completeLine(MakefileLine &line);
+    void completePreprocessingDirectiveLine(MakefileLine &line);
     bool parseMacro(const QString& line);
     bool parsePreprocessingDirective(const QString& line);
     QString findIncludeFile(const QString &filePathToInclude);
diff --git a/tests/tests.cpp b/tests/tests.cpp
index 668baeb..0d8ee79 100644
--- a/tests/tests.cpp
+++ b/tests/tests.cpp
@@ -313,7 +313,6 @@ void Tests::preprocessorInvalidExpressions()
 
 void Tests::conditionals()
 {
-    QEXPECT_FAIL("", "QTCREATORBUG-8621", Continue);
     QVERIFY( openMakefile(QLatin1String("conditionals.mk")) );
     QScopedPointer<Makefile> mkfile(m_makefileFactory->makefile());
     QVERIFY(mkfile);
@@ -328,9 +327,7 @@ void Tests::conditionals()
     QCOMPARE(macroTable->macroValue("TEST6"), QLatin1String("true"));
     QCOMPARE(macroTable->macroValue("TEST7"), QLatin1String("true"));
     QCOMPARE(macroTable->macroValue("TEST8"), QLatin1String("true"));
-    QEXPECT_FAIL("", "QTCREATORBUG-8621", Continue);
     QCOMPARE(macroTable->macroValue("TEST9"), QLatin1String("foo  bar  baz"));
-    QEXPECT_FAIL("", "QTCREATORBUG-8621", Continue);
     QCOMPARE(macroTable->macroValue("TEST10"), QLatin1String("foo  bar  boo  hoo"));
 }