Changeset View
Standalone View
plugins/clang/duchain/parsesession.cpp
| Show First 20 Lines • Show All 85 Lines • ▼ Show 20 Line(s) | |||||
| 86 | #endif | 86 | #endif | ||
| 87 | } | 87 | } | ||
| 88 | 88 | | |||
| 89 | } | 89 | } | ||
| 90 | 90 | | |||
| 91 | QVector<QByteArray> argsForSession(const QString& path, ParseSessionData::Options options, const ParserSettings& parserSettings) | 91 | QVector<QByteArray> argsForSession(const QString& path, ParseSessionData::Options options, const ParserSettings& parserSettings) | ||
| 92 | { | 92 | { | ||
| 93 | QMimeDatabase db; | 93 | QMimeDatabase db; | ||
| 94 | if(db.mimeTypeForFile(path).name() == QStringLiteral("text/x-objcsrc")) { | 94 | QString mimeType = db.mimeTypeForFile(path).name(); | ||
mwolff: `const auto mimeType = ...`
and then move down to where this is actually used | |||||
| 95 | return {QByteArrayLiteral("-xobjective-c++")}; | | |||
| 96 | } | | |||
| 97 | | ||||
| 98 | // TODO: No proper mime type detection possible yet | 95 | // TODO: No proper mime type detection possible yet | ||
| 99 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=26913 | 96 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=26913 | ||
| 100 | if (path.endsWith(QLatin1String(".cl"), Qt::CaseInsensitive)) { | 97 | if (path.endsWith(QLatin1String(".cl"), Qt::CaseInsensitive)) { | ||
| 101 | return {QByteArrayLiteral("-xcl")}; | 98 | return {QByteArrayLiteral("-xcl")}; | ||
| 102 | } | 99 | } | ||
| 103 | 100 | | |||
| 104 | // TODO: No proper mime type detection possible yet | 101 | // TODO: No proper mime type detection possible yet | ||
| 105 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=23700 | 102 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=23700 | ||
| 106 | if (path.endsWith(QLatin1String(".cu"), Qt::CaseInsensitive)) { | 103 | if (path.endsWith(QLatin1String(".cu"), Qt::CaseInsensitive)) { | ||
| 107 | return {QByteArrayLiteral("-xcuda")}; | 104 | return {QByteArrayLiteral("-xcuda")}; | ||
| 108 | } | 105 | } | ||
| 109 | 106 | | |||
| 110 | if (parserSettings.parserOptions.isEmpty()) { | 107 | QByteArray objcOption; | ||
| 108 | if (mimeType == QStringLiteral("text/x-objcsrc")) { | ||||
| 109 | objcOption = QByteArrayLiteral(" -ObjC"); | ||||
| 110 | } else if (mimeType == QStringLiteral("text/x-objc++src")) { | ||||
| 111 | objcOption = QByteArrayLiteral("-xobjective-c++"); | ||||
| 112 | } else if (parserSettings.parserOptions.isEmpty()) { | ||||
| 111 | // The parserOptions can be empty for some unit tests that use ParseSession directly | 113 | // The parserOptions can be empty for some unit tests that use ParseSession directly | ||
This seems to be relevant for unit tests, why do you do the distinction here? I think it should be done down where my other comment is. aaronpuchert: This seems to be relevant for unit tests, why do you do the distinction here? I think it should… | |||||
I didn't want to presume that parserSettings.parserOptions can never be empty for other reasons, and didn't want to change the logic completely where ObjC presence would override parserOptions.isEmpty() (which causes a return). rjvbb: I didn't want to presume that parserSettings.parserOptions can never be empty for other reasons… | |||||
| 112 | auto defaultArguments = ClangSettingsManager::self()->parserSettings(path).toClangAPI(); | 114 | auto defaultArguments = ClangSettingsManager::self()->parserSettings(path).toClangAPI(); | ||
| 113 | 115 | | |||
| 114 | defaultArguments.append(QByteArrayLiteral("-nostdinc")); | 116 | defaultArguments.append(QByteArrayLiteral("-nostdinc")); | ||
| 115 | defaultArguments.append(QByteArrayLiteral("-nostdinc++")); | 117 | defaultArguments.append(QByteArrayLiteral("-nostdinc++")); | ||
| 116 | defaultArguments.append(QByteArrayLiteral("-xc++")); | 118 | defaultArguments.append(QByteArrayLiteral("-xc++")); | ||
by moving down the check you break this path, but apparently that will only show up in unit tests mwolff: by moving down the check you break this path, but apparently that will only show up in unit… | |||||
Sorry, comments and code seem to have gone out of sync so this is a bit cryptic. What path do I break? I can see how unittests that didn't set parserOptions will not get relevant options for ObjC so I've tried to address that. rjvbb: Sorry, comments and code seem to have gone out of sync so this is a bit cryptic. What path do I… | |||||
| 117 | 119 | | |||
| 118 | sanitizeArguments(defaultArguments); | 120 | sanitizeArguments(defaultArguments); | ||
| 119 | return defaultArguments; | 121 | return defaultArguments; | ||
yep remove this, I hope it's all fine (did you check that there are no new tests failures?) mwolff: yep remove this, I hope it's all fine (did you check that there are no new tests failures?) | |||||
I don't see any, no. Are there expected failures that are not labelled as such, or any tests I should check in particular? I only get a cuda-related failure in the test_files test, and that's probably unrelated to my changes and due to error: cannot find CUDA installation. Provide its path via --cuda-path, or pass -nocudainc to build without CUDA includes. error: cannot find libdevice for sm_20. Provide path to different CUDA installation via --cuda-path, or pass -nocudalib to build without linking with libdevice. error: cannot find CUDA installation. Provide its path via --cuda-path, or pass -nocudainc to build without CUDA includes. rjvbb: I don't see any, no. Are there expected failures that are not labelled as such, or any tests I… | |||||
| 120 | } | 122 | } | ||
| 121 | 123 | | |||
| 122 | auto result = parserSettings.toClangAPI(); | 124 | auto result = parserSettings.toClangAPI(); | ||
| 123 | result.append(QByteArrayLiteral("-nostdinc")); | 125 | result.append(QByteArrayLiteral("-nostdinc")); | ||
| 124 | if (parserSettings.isCpp()) { | 126 | if (parserSettings.isCpp()) { | ||
| 125 | result.append(QByteArrayLiteral("-nostdinc++")); | 127 | result.append(QByteArrayLiteral("-nostdinc++")); | ||
| 126 | } | 128 | } | ||
| 127 | 129 | | |||
| 128 | if (options & ParseSessionData::PrecompiledHeader) { | 130 | if (options & ParseSessionData::PrecompiledHeader) { | ||
| 129 | result.append(parserSettings.isCpp() ? QByteArrayLiteral("-xc++-header") : QByteArrayLiteral("-xc-header")); | 131 | result.append(parserSettings.isCpp() ? QByteArrayLiteral("-xc++-header") : QByteArrayLiteral("-xc-header")); | ||
| 130 | 132 | | |||
| 131 | sanitizeArguments(result); | 133 | sanitizeArguments(result); | ||
| 132 | return result; | 134 | return result; | ||
| 133 | } | 135 | } | ||
| 134 | 136 | | |||
| 137 | if (!objcOption.isEmpty()) { | ||||
| 138 | result.append(objcOption); | ||||
| 139 | } else { | ||||
| 135 | result.append(parserSettings.isCpp() ? QByteArrayLiteral("-xc++") : QByteArrayLiteral("-xc")); | 140 | result.append(parserSettings.isCpp() ? QByteArrayLiteral("-xc++") : QByteArrayLiteral("-xc")); | ||
| 141 | } | ||||
... but do we really want to move these checks down here? is it OK to pass the -nostdinc command e.g. for objc? mwolff: ... but do we really want to move these checks down here? is it OK to pass the -nostdinc… | |||||
It is; ObjC is a superset of C and ObjC++ is a superset of C++ so to the best of my knownledge compiler options for the parent language are OK. I suppose the alternative is to move the checks upwards so that the options are prepended to the session arguments list, which will be a bit easier with the newly reorganised code. I have slight preference for appending because that ensures the parser is always switched to ObjC mode when the mimetype says it should. rjvbb: It is; ObjC is a superset of C and ObjC++ is a superset of C++ so to the best of my knownledge… | |||||
| 136 | 142 | | |||
aaronpuchert: Maybe the time has come to make this a `switch` statement. | |||||
Maybe as a separate, later change, or is there already a state or language variable that is suitable an argument for a switch statement? rjvbb: Maybe as a separate, later change, or is there already a state or language variable that is… | |||||
Thanks, that's exactly what I meant. It doesn't literally have to be a switch statement. (Although it would be a nice refactoring to add an enumeration later.) aaronpuchert: Thanks, that's exactly what I meant. It doesn't literally have to be a switch statement. | |||||
| 137 | sanitizeArguments(result); | 143 | sanitizeArguments(result); | ||
| 138 | return result; | 144 | return result; | ||
| 139 | } | 145 | } | ||
| 140 | 146 | | |||
| 141 | void addIncludes(QVector<const char*>* args, QVector<QByteArray>* otherArgs, | 147 | void addIncludes(QVector<const char*>* args, QVector<QByteArray>* otherArgs, | ||
| 142 | const Path::List& includes, const char* cliSwitch) | 148 | const Path::List& includes, const char* cliSwitch) | ||
| 143 | { | 149 | { | ||
| 144 | for (const Path& url : includes) { | 150 | for (const Path& url : includes) { | ||
| ▲ Show 20 Lines • Show All 391 Lines • Show Last 20 Lines | |||||
const auto mimeType = ...
and then move down to where this is actually used