Clang: Pass on file paths with native separators

libclang 3.8 seems to be sensitive to file paths separators [1]. On Windows,
this led to not updated document annotations and/or crashes after reparsing.

When passing file paths to libclang, convert to native separators.
When getting file paths from libclang, convert back.

This handles:
 * main file path
 * file paths of the unsaved files
 * -I<DIR> arguments, the resource path (for builtins) and the paths to the
   wrapped qt headers
 * included header files from libclang
 * source locations from libclang

Also, minimize the conversion in SourceLocation to a minimum by making
filePath() lazy.

[1] https://llvm.org/bugs/show_bug.cgi?id=28381

Change-Id: If5866f34a6fdc6b34b16c022d3988e8e6eae2a0a
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/clangcodemodel/clangutils.cpp

@@ -98,7 +98,7 @@ public:
 
         optionsBuilder.addPredefinedMacrosAndHeaderPathsOptions();
         optionsBuilder.addWrappedQtHeadersIncludePath();
-        optionsBuilder.addHeaderPathOptions();
+        optionsBuilder.addHeaderPathOptions(/*addAsNativePath*/ true);
         optionsBuilder.addProjectConfigFileInclude();
 
         optionsBuilder.addMsvcCompatibilityVersion();
@@ -149,19 +149,20 @@ private:
         static const QString resourceDir = getResourceDir();
         if (!resourceDir.isEmpty()) {
             add(QLatin1String("-nostdlibinc"));
-            add(QLatin1String("-I") + resourceDir);
+            add(QLatin1String("-I") + QDir::toNativeSeparators(resourceDir));
             add(QLatin1String("-undef"));
         }
     }
 
     void addWrappedQtHeadersIncludePath()
     {
-        static const QString wrappedQtHeaders = ICore::instance()->resourcePath()
+        static const QString wrappedQtHeadersPath = ICore::instance()->resourcePath()
                 + QLatin1String("/cplusplus/wrappedQtHeaders");
 
         if (m_projectPart.qtVersion != ProjectPart::NoQt) {
-            add(QLatin1String("-I") + wrappedQtHeaders);
+            const QString wrappedQtCoreHeaderPath = wrappedQtHeadersPath + QLatin1String("/QtCore");
-            add(QLatin1String("-I") + wrappedQtHeaders + QLatin1String("/QtCore"));
+            add(QLatin1String("-I") + QDir::toNativeSeparators(wrappedQtHeadersPath));
+            add(QLatin1String("-I") + QDir::toNativeSeparators(wrappedQtCoreHeaderPath));
         }
     }
 
@@ -169,7 +170,7 @@ private:
     {
         if (!m_projectPart.projectConfigFile.isEmpty()) {
             add(QLatin1String("-include"));
-            add(m_projectPart.projectConfigFile);
+            add(QDir::toNativeSeparators(m_projectPart.projectConfigFile));
         }
     }
 

src/plugins/cpptools/compileroptionsbuilder.cpp

@@ -27,6 +27,8 @@
 
 #include <projectexplorer/projectexplorerconstants.h>
 
+#include <QDir>
+
 namespace CppTools {
 
 CompilerOptionsBuilder::CompilerOptionsBuilder(const ProjectPart &projectPart)
@@ -98,7 +100,7 @@ void CompilerOptionsBuilder::enableExceptions()
     add(QLatin1String("-fexceptions"));
 }
 
-void CompilerOptionsBuilder::addHeaderPathOptions()
+void CompilerOptionsBuilder::addHeaderPathOptions(bool addAsNativePath)
 {
     typedef ProjectPartHeaderPath HeaderPath;
     const QString defaultPrefix = includeOption();
@@ -124,7 +126,10 @@ void CompilerOptionsBuilder::addHeaderPathOptions()
             break;
         }
 
-        result.append(prefix + headerPath.path);
+        QString path = prefix + headerPath.path;
+        path = addAsNativePath ? QDir::toNativeSeparators(path) : path;
+
+        result.append(path);
     }
 
     m_options.append(result);

src/plugins/cpptools/compileroptionsbuilder.h

@@ -46,7 +46,7 @@ public:
     // Add options based on project part
     virtual void addTargetTriple();
     virtual void enableExceptions();
-    void addHeaderPathOptions();
+    void addHeaderPathOptions(bool addAsNativePath = false);
     void addToolchainAndProjectDefines();
     virtual void addLanguageOption(ProjectFile::Kind fileKind);
     virtual void addOptionsForLanguage(bool checkForBorlandExtensions = true);

src/tools/clangbackend/ipcsource/clangbackendclangipc-source.pri

@@ -34,7 +34,8 @@ HEADERS += $$PWD/clangipcserver.h \
     $$PWD/highlightingmark.h \
     $$PWD/highlightingmarks.h \
     $$PWD/highlightingmarksiterator.h \
-    $$PWD/utf8positionfromlinecolumn.h
+    $$PWD/utf8positionfromlinecolumn.h \
+    $$PWD/clangfilepath.h
 
 SOURCES += $$PWD/clangipcserver.cpp \
     $$PWD/codecompleter.cpp \
@@ -68,4 +69,5 @@ SOURCES += $$PWD/clangipcserver.cpp \
     $$PWD/clangtype.cpp \
     $$PWD/highlightingmark.cpp \
     $$PWD/highlightingmarks.cpp \
-    $$PWD/utf8positionfromlinecolumn.cpp
+    $$PWD/utf8positionfromlinecolumn.cpp \
+    $$PWD/clangfilepath.cpp

src/tools/clangbackend/ipcsource/clangfilepath.cpp

@@ -0,0 +1,87 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#include "clangfilepath.h"
+
+#include <utf8string.h>
+
+#include <QByteArray>
+
+// Parameterized QDir::toNativeSeparators/-fromNativeSeparators for QByteArray
+static inline QByteArray replace(const QByteArray &pathName, char toReplace, char replacement)
+{
+    int i = pathName.indexOf(toReplace);
+    if (i != -1) {
+        QByteArray n(pathName);
+
+        char * const data = n.data();
+        data[i++] = replacement;
+
+        for (; i < n.length(); ++i) {
+            if (data[i] == toReplace)
+                data[i] = replacement;
+        }
+
+        return n;
+    }
+
+    return pathName;
+}
+
+static QByteArray fromNativeSeparatorsForQByteArray(const QByteArray &pathName)
+{
+#if defined(Q_OS_WIN)
+    return replace(pathName, '\\', '/');
+#else
+    return pathName;
+#endif
+}
+
+static QByteArray toNativeSeparatorsForQByteArray(const QByteArray &pathName)
+{
+#if defined(Q_OS_WIN)
+    return replace(pathName, '/', '\\');
+#else
+    return pathName;
+#endif
+}
+
+namespace ClangBackEnd {
+
+Utf8String FilePath::fromNativeSeparators(const Utf8String &pathName)
+{
+    const QByteArray pathNameAsByteArray = pathName.toByteArray();
+
+    return Utf8String::fromUtf8(fromNativeSeparatorsForQByteArray(pathNameAsByteArray));
+}
+
+Utf8String FilePath::toNativeSeparators(const Utf8String &pathName)
+{
+    const QByteArray pathNameAsByteArray = pathName.toByteArray();
+
+    return Utf8String::fromUtf8(toNativeSeparatorsForQByteArray(pathNameAsByteArray));
+}
+
+} // namespace ClangBackEnd

src/tools/clangbackend/ipcsource/clangfilepath.h

@@ -0,0 +1,38 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#pragma once
+
+class Utf8String;
+
+namespace ClangBackEnd {
+
+class FilePath {
+public:
+    static Utf8String fromNativeSeparators(const Utf8String &pathName);
+    static Utf8String toNativeSeparators(const Utf8String &pathName);
+};
+
+} // namespace ClangBackEnd

src/tools/clangbackend/ipcsource/clangtranslationunit.cpp

@@ -26,6 +26,7 @@
 #include "clangtranslationunit.h"
 
 #include "cursor.h"
+#include "clangfilepath.h"
 #include "clangstring.h"
 #include "codecompleter.h"
 #include "commandlinearguments.h"
@@ -475,7 +476,8 @@ void TranslationUnit::includeCallback(CXFile included_file,
 
     TranslationUnit *translationUnit = static_cast<TranslationUnit*>(clientData);
 
-    translationUnit->d->dependedFilePaths.insert(includeFilePath);
+    const Utf8String normalizedFilePath = FilePath::fromNativeSeparators(includeFilePath);
+    translationUnit->d->dependedFilePaths.insert(normalizedFilePath);
 }
 
 UnsavedFiles &TranslationUnit::unsavedFiles() const

src/tools/clangbackend/ipcsource/codecompleter.cpp

@@ -25,6 +25,7 @@
 
 #include "codecompleter.h"
 
+#include "clangfilepath.h"
 #include "clangcodecompleteresults.h"
 #include "clangstring.h"
 #include "cursor.h"
@@ -83,8 +84,10 @@ ClangCodeCompleteResults CodeCompleter::complete(uint line,
                                                  CXUnsavedFile *unsavedFiles,
                                                  unsigned unsavedFileCount)
 {
+    const Utf8String nativeFilePath = FilePath::toNativeSeparators(translationUnit.filePath());
+
     return clang_codeCompleteAt(translationUnit.cxTranslationUnitWithoutReparsing(),
-                                translationUnit.filePath().constData(),
+                                nativeFilePath.constData(),
                                 line,
                                 column,
                                 unsavedFiles,

src/tools/clangbackend/ipcsource/commandlinearguments.cpp

@@ -25,8 +25,12 @@
 
 #include "commandlinearguments.h"
 
+#include "clangfilepath.h"
+
 #include <utf8string.h>
 
+#include <QByteArray>
+
 #include <iostream>
 
 namespace ClangBackEnd {
@@ -46,7 +50,8 @@ CommandLineArguments::CommandLineArguments(const char *filePath,
         m_arguments.push_back(argument.constData());
     if (addVerboseOption)
         m_arguments.push_back("-v");
-    m_arguments.push_back(filePath);
+    m_nativeFilePath = FilePath::toNativeSeparators(Utf8String::fromUtf8(filePath));
+    m_arguments.push_back(m_nativeFilePath.constData());
 }
 
 const char * const *CommandLineArguments::data() const

src/tools/clangbackend/ipcsource/commandlinearguments.h

@@ -46,6 +46,7 @@ public:
     void print() const;
 
 private:
+    Utf8String m_nativeFilePath;
     std::vector<const char *> m_arguments;
 };
 

src/tools/clangbackend/ipcsource/sourcelocation.cpp

@@ -25,6 +25,7 @@
 
 #include "sourcelocation.h"
 
+#include "clangfilepath.h"
 #include "clangstring.h"
 #include "clangtranslationunit.h"
 
@@ -42,6 +43,12 @@ SourceLocation::SourceLocation()
 
 const Utf8String &SourceLocation::filePath() const
 {
+    if (isFilePathNormalized_)
+        return filePath_;
+
+    isFilePathNormalized_ = true;
+    filePath_ = FilePath::fromNativeSeparators(filePath_);
+
     return filePath_;
 }
 
@@ -62,7 +69,7 @@ uint SourceLocation::offset() const
 
 SourceLocationContainer SourceLocation::toSourceLocationContainer() const
 {
-    return SourceLocationContainer(filePath_, line_, column_);
+    return SourceLocationContainer(filePath(), line_, column_);
 }
 
 SourceLocation::SourceLocation(CXSourceLocation cxSourceLocation)
@@ -77,6 +84,7 @@ SourceLocation::SourceLocation(CXSourceLocation cxSourceLocation)
                           &offset_);
 
     filePath_ = ClangString(clang_getFileName(cxFile));
+    isFilePathNormalized_ = false;
 }
 
 SourceLocation::SourceLocation(CXTranslationUnit cxTranslationUnit,
@@ -90,7 +98,8 @@ SourceLocation::SourceLocation(CXTranslationUnit cxTranslationUnit,
                                          column)),
       filePath_(filePath),
       line_(line),
-      column_(column)
+      column_(column),
+      isFilePathNormalized_(true)
 {
     clang_getFileLocation(cxSourceLocation, 0, 0, 0, &offset_);
 }

src/tools/clangbackend/ipcsource/sourcelocation.h

@@ -63,10 +63,11 @@ private:
 
 private:
    CXSourceLocation cxSourceLocation;
-   Utf8String filePath_;
+   mutable Utf8String filePath_;
    uint line_ = 0;
    uint column_ = 0;
    uint offset_ = 0;
+   mutable bool isFilePathNormalized_ = true;
 };
 
 bool operator==(const SourceLocation &first, const SourceLocation &second);

src/tools/clangbackend/ipcsource/unsavedfile.cpp

@@ -25,6 +25,7 @@
 
 #include "unsavedfile.h"
 
+#include "clangfilepath.h"
 #include "utf8string.h"
 #include "utf8positionfromlinecolumn.h"
 
@@ -40,10 +41,12 @@ UnsavedFile::UnsavedFile()
 
 UnsavedFile::UnsavedFile(const Utf8String &filePath, const Utf8String &fileContent)
 {
-    char *cxUnsavedFilePath = new char[filePath.byteSize() + 1];
+    const Utf8String nativeFilePath = FilePath::toNativeSeparators(filePath);
+
+    char *cxUnsavedFilePath = new char[nativeFilePath.byteSize() + 1];
     char *cxUnsavedFileContent = new char[fileContent.byteSize() + 1];
 
-    std::memcpy(cxUnsavedFilePath, filePath.constData(), filePath.byteSize() + 1);
+    std::memcpy(cxUnsavedFilePath, nativeFilePath.constData(), nativeFilePath.byteSize() + 1);
     std::memcpy(cxUnsavedFileContent, fileContent.constData(), fileContent.byteSize() + 1);
 
     cxUnsavedFile = CXUnsavedFile{cxUnsavedFilePath,
@@ -66,7 +69,14 @@ UnsavedFile &UnsavedFile::operator=(UnsavedFile &&other) Q_DECL_NOEXCEPT
     return *this;
 }
 
-const char *UnsavedFile::filePath() const
+Utf8String UnsavedFile::filePath() const
+{
+    const Utf8String nativeFilePathAsUtf8String = Utf8String::fromUtf8(nativeFilePath());
+
+    return FilePath::fromNativeSeparators(nativeFilePathAsUtf8String);
+}
+
+const char *UnsavedFile::nativeFilePath() const
 {
     return cxUnsavedFile.Filename;
 }
@@ -105,7 +115,7 @@ bool UnsavedFile::replaceAt(uint position, uint length, const Utf8String &replac
         Utf8String modifiedContent(cxUnsavedFile.Contents, cxUnsavedFile.Length);
         modifiedContent.replace(int(position), int(length), replacement);
 
-        *this = UnsavedFile(Utf8String::fromUtf8(filePath()), modifiedContent);
+        *this = UnsavedFile(Utf8String::fromUtf8(nativeFilePath()), modifiedContent);
 
         return true;
     }

src/tools/clangbackend/ipcsource/unsavedfile.h

@@ -52,7 +52,8 @@ public:
     UnsavedFile(UnsavedFile &&other) Q_DECL_NOEXCEPT;
     UnsavedFile &operator=(UnsavedFile &&other) Q_DECL_NOEXCEPT;
 
-    const char *filePath() const;
+    Utf8String filePath() const;
+    const char *nativeFilePath() const;
 
     // 1-based line and column
     uint toUtf8Position(uint line, uint column, bool *ok) const;

tests/unit/unittest/clangcodecompleteresultstest.cpp

@@ -24,6 +24,7 @@
 ****************************************************************************/
 
 #include <clangcodecompleteresults.h>
+#include <clangfilepath.h>
 #include <projectpart.h>
 #include <projects.h>
 #include <clangtranslationunit.h>
@@ -41,6 +42,7 @@
 namespace {
 
 using ClangBackEnd::ClangCodeCompleteResults;
+using ClangBackEnd::FilePath;
 using ClangBackEnd::TranslationUnit;
 using ClangBackEnd::UnsavedFiles;
 using ClangBackEnd::ProjectPart;
@@ -55,7 +57,8 @@ TEST(ClangCodeCompleteResults, GetData)
                                     projectPart,
                                     Utf8StringVector(),
                                     translationUnits);
-    CXCodeCompleteResults *cxCodeCompleteResults = clang_codeCompleteAt(translationUnit.cxTranslationUnit(), translationUnit.filePath().constData(), 49, 1, 0, 0, 0);
+    Utf8String nativeFilePath = FilePath::toNativeSeparators(translationUnit.filePath());
+    CXCodeCompleteResults *cxCodeCompleteResults = clang_codeCompleteAt(translationUnit.cxTranslationUnit(), nativeFilePath.constData(), 49, 1, 0, 0, 0);
 
     ClangCodeCompleteResults codeCompleteResults(cxCodeCompleteResults);
 
@@ -81,7 +84,8 @@ TEST(ClangCodeCompleteResults, MoveClangCodeCompleteResults)
                                     projectPart,
                                     Utf8StringVector(),
                                     translationUnits);
-    CXCodeCompleteResults *cxCodeCompleteResults = clang_codeCompleteAt(translationUnit.cxTranslationUnit(), translationUnit.filePath().constData(), 49, 1, 0, 0, 0);
+    Utf8String nativeFilePath = FilePath::toNativeSeparators(translationUnit.filePath());
+    CXCodeCompleteResults *cxCodeCompleteResults = clang_codeCompleteAt(translationUnit.cxTranslationUnit(), nativeFilePath.constData(), 49, 1, 0, 0, 0);
 
     ClangCodeCompleteResults codeCompleteResults(cxCodeCompleteResults);
 

tests/unit/unittest/codecompletionsextractortest.cpp

@@ -24,6 +24,7 @@
 ****************************************************************************/
 
 #include <clangcodecompleteresults.h>
+#include <clangfilepath.h>
 #include <codecompletionsextractor.h>
 #include <filecontainer.h>
 #include <projectpart.h>
@@ -44,6 +45,7 @@
 
 using ClangBackEnd::CodeCompletionsExtractor;
 using ClangBackEnd::ClangCodeCompleteResults;
+using ClangBackEnd::FilePath;
 using ClangBackEnd::TranslationUnit;
 using ClangBackEnd::CodeCompletion;
 using ClangBackEnd::UnsavedFiles;
@@ -139,8 +141,10 @@ const ClangBackEnd::FileContainer unsavedDataFileContainer(const char *filePath,
 
 ClangCodeCompleteResults getResults(const TranslationUnit &translationUnit, uint line, uint column = 1)
 {
+    Utf8String nativeFilePath = FilePath::toNativeSeparators(translationUnit.filePath());
+
     return ClangCodeCompleteResults(clang_codeCompleteAt(translationUnit.cxTranslationUnit(),
-                                                         translationUnit.filePath().constData(),
+                                                         nativeFilePath.constData(),
                                                          line,
                                                          column,
                                                          translationUnit.cxUnsavedFiles(),

tests/unit/unittest/translationunittest.cpp

@@ -23,6 +23,7 @@
 **
 ****************************************************************************/
 
+#include <clangfilepath.h>
 #include <commandlinearguments.h>
 #include <diagnosticset.h>
 #include <highlightingmarks.h>
@@ -52,6 +53,7 @@
 #include <thread>
 
 using ClangBackEnd::FileContainer;
+using ClangBackEnd::FilePath;
 using ClangBackEnd::TranslationUnit;
 using ClangBackEnd::UnsavedFiles;
 using ClangBackEnd::ProjectPart;
@@ -156,9 +158,10 @@ TEST_F(TranslationUnit, ResetedTranslationUnitIsNull)
 
 TEST_F(TranslationUnit, LastCommandLineArgumentIsFilePath)
 {
+    const Utf8String nativeFilePath = FilePath::toNativeSeparators(translationUnitFilePath);
     const auto arguments = translationUnit.commandLineArguments();
 
-    ASSERT_THAT(arguments.at(arguments.count() - 1), Eq(translationUnitFilePath));
+    ASSERT_THAT(arguments.at(arguments.count() - 1), Eq(nativeFilePath));
 }
 
 TEST_F(TranslationUnit, TimeStampForProjectPartChangeIsUpdatedAsNewCxTranslationUnitIsGenerated)

tests/unit/unittest/unsavedfiletest.cpp

@@ -28,9 +28,11 @@
 #include "gmock/gmock.h"
 #include "gtest-qt-printing.h"
 
+#include <clangfilepath.h>
 #include <unsavedfile.h>
 #include <unsavedfiles.h>
 
+using ClangBackEnd::FilePath;
 using ClangBackEnd::UnsavedFile;
 using ClangBackEnd::UnsavedFiles;
 
@@ -61,6 +63,8 @@ protected:
     Utf8String otherFilePath = Utf8StringLiteral("otherpath");
     Utf8String otherFileContent = Utf8StringLiteral("othercontent");
 
+    Utf8String absoluteFilePath = Utf8StringLiteral(TESTDATA_DIR"/file.cpp");
+
     uint aLength = 2;
     Utf8String aReplacement = Utf8StringLiteral("replacement");
 };
@@ -126,6 +130,21 @@ TEST_F(UnsavedFile, AssignMoveFromIsSwapped)
                                          otherFileContent.byteSize()));
 }
 
+TEST_F(UnsavedFile, FilePath)
+{
+    ::UnsavedFile unsavedFile(absoluteFilePath, QStringLiteral(""));
+
+    ASSERT_THAT(unsavedFile.filePath(), Eq(absoluteFilePath));
+}
+
+TEST_F(UnsavedFile, NativeFilePath)
+{
+    ::UnsavedFile unsavedFile(absoluteFilePath, QStringLiteral(""));
+    const Utf8String nativeFilePath = FilePath::toNativeSeparators(absoluteFilePath);
+
+    ASSERT_THAT(unsavedFile.nativeFilePath(), Eq(nativeFilePath));
+}
+
 TEST_F(UnsavedFile, DoNotReplaceWithOffsetZeroInEmptyContent)
 {
     ::UnsavedFile unsavedFile(filePath, QStringLiteral(""));