Changeset View
Standalone View
plugins/clang/duchain/parsesession.cpp
| Show First 20 Lines • Show All 83 Lines • ▼ Show 20 Line(s) | 81 | #if CINDEX_VERSION_MINOR < 100 // FIXME https://bugs.llvm.org/show_bug.cgi?id=35333 | |||
|---|---|---|---|---|---|
| 84 | } | 84 | } | ||
| 85 | #endif | 85 | #endif | ||
| 86 | } | 86 | } | ||
| 87 | 87 | | |||
| 88 | } | 88 | } | ||
| 89 | 89 | | |||
| 90 | QVector<QByteArray> argsForSession(const QString& path, ParseSessionData::Options options, const ParserSettings& parserSettings) | 90 | QVector<QByteArray> argsForSession(const QString& path, ParseSessionData::Options options, const ParserSettings& parserSettings) | ||
| 91 | { | 91 | { | ||
| 92 | QMimeDatabase db; | 92 | QMimeDatabase db; | ||
| 93 | if(db.mimeTypeForFile(path).name() == QStringLiteral("text/x-objcsrc")) { | | |||
| 94 | return {QByteArrayLiteral("-xobjective-c++")}; | | |||
| 95 | } | | |||
| 96 | | ||||
| 97 | // TODO: No proper mime type detection possible yet | 93 | // TODO: No proper mime type detection possible yet | ||
mwolff: `const auto mimeType = ...`
and then move down to where this is actually used | |||||
| 98 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=26913 | 94 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=26913 | ||
| 99 | if (path.endsWith(QLatin1String(".cl"), Qt::CaseInsensitive)) { | 95 | if (path.endsWith(QLatin1String(".cl"), Qt::CaseInsensitive)) { | ||
| 100 | return {QByteArrayLiteral("-xcl")}; | 96 | return {QByteArrayLiteral("-xcl")}; | ||
| 101 | } | 97 | } | ||
| 102 | 98 | | |||
| 103 | // TODO: No proper mime type detection possible yet | 99 | // TODO: No proper mime type detection possible yet | ||
| 104 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=23700 | 100 | // cf. https://bugs.freedesktop.org/show_bug.cgi?id=23700 | ||
| 105 | if (path.endsWith(QLatin1String(".cu"), Qt::CaseInsensitive) || | 101 | if (path.endsWith(QLatin1String(".cu"), Qt::CaseInsensitive) || | ||
| 106 | path.endsWith(QLatin1String(".cuh"), Qt::CaseInsensitive)) { | 102 | path.endsWith(QLatin1String(".cuh"), Qt::CaseInsensitive)) { | ||
| 107 | auto result = parserSettings.toClangAPI(); | 103 | auto result = parserSettings.toClangAPI(); | ||
| 108 | result.append(QByteArrayLiteral("-xcuda")); | 104 | result.append(QByteArrayLiteral("-xcuda")); | ||
| 109 | return result; | 105 | return result; | ||
| 110 | } | 106 | } | ||
| 111 | 107 | | |||
| 108 | auto result = parserSettings.toClangAPI(); | ||||
| 112 | if (parserSettings.parserOptions.isEmpty()) { | 109 | if (parserSettings.parserOptions.isEmpty()) { | ||
| 113 | // The parserOptions can be empty for some unit tests that use ParseSession directly | 110 | // The parserOptions can be empty for some unit tests that use ParseSession directly | ||
| 114 | auto defaultArguments = ClangSettingsManager::self()->parserSettings(path).toClangAPI(); | 111 | result = ClangSettingsManager::self()->parserSettings(path).toClangAPI(); | ||
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… | |||||
| 115 | | ||||
| 116 | defaultArguments.append(QByteArrayLiteral("-nostdinc")); | | |||
| 117 | defaultArguments.append(QByteArrayLiteral("-nostdinc++")); | | |||
| 118 | defaultArguments.append(QByteArrayLiteral("-xc++")); | | |||
| 119 | 112 | | |||
| 120 | sanitizeArguments(defaultArguments); | 113 | // FIXME: remove commented lines if the new flow is ratified, else revert change. | ||
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… | |||||
| 121 | return defaultArguments; | 114 | // defaultArguments.append(QByteArrayLiteral("-nostdinc")); | ||
| 115 | // defaultArguments.append(QByteArrayLiteral("-nostdinc++")); | ||||
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… | |||||
| 116 | // defaultArguments.append(QByteArrayLiteral("-xc++")); | ||||
| 117 | // | ||||
| 118 | // sanitizeArguments(defaultArguments); | ||||
| 119 | // return defaultArguments; | ||||
| 122 | } | 120 | } | ||
| 123 | 121 | | |||
| 124 | auto result = parserSettings.toClangAPI(); | | |||
| 125 | result.append(QByteArrayLiteral("-nostdinc")); | 122 | result.append(QByteArrayLiteral("-nostdinc")); | ||
| 126 | if (parserSettings.isCpp()) { | 123 | if (parserSettings.isCpp()) { | ||
| 127 | result.append(QByteArrayLiteral("-nostdinc++")); | 124 | result.append(QByteArrayLiteral("-nostdinc++")); | ||
| 128 | } | 125 | } | ||
| 129 | 126 | | |||
| 130 | if (options & ParseSessionData::PrecompiledHeader) { | 127 | if (options & ParseSessionData::PrecompiledHeader) { | ||
| 131 | result.append(parserSettings.isCpp() ? QByteArrayLiteral("-xc++-header") : QByteArrayLiteral("-xc-header")); | 128 | result.append(parserSettings.isCpp() ? QByteArrayLiteral("-xc++-header") : QByteArrayLiteral("-xc-header")); | ||
| 132 | 129 | | |||
| 133 | sanitizeArguments(result); | 130 | sanitizeArguments(result); | ||
| 134 | return result; | 131 | return result; | ||
| 135 | } | 132 | } | ||
| 136 | 133 | | |||
| 137 | result.append(parserSettings.isCpp() ? QByteArrayLiteral("-xc++") : QByteArrayLiteral("-xc")); | 134 | // check the C family membership and set the appropriate mode | ||
| 135 | // overriding any inappropriate modes set earlier in the args list. | ||||
| 136 | const auto mimeType = db.mimeTypeForFile(path).name(); | ||||
| 137 | if (mimeType == QStringLiteral("text/x-objc++src")) { | ||||
| 138 | result.append(QByteArrayLiteral("-xobjective-c++")); | ||||
| 139 | } else if (mimeType == QStringLiteral("text/x-objcsrc")) { | ||||
| 140 | result.append(QByteArrayLiteral(" -ObjC")); | ||||
| 141 | } else if (parserSettings.isCpp()) { | ||||
| 142 | result.append(QByteArrayLiteral("-xc++")); | ||||
| 143 | } else { | ||||
| 144 | result.append(QByteArrayLiteral("-xc")); | ||||
| 145 | } | ||||
... 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… | |||||
| 138 | 146 | | |||
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. | |||||
| 139 | sanitizeArguments(result); | 147 | sanitizeArguments(result); | ||
| 140 | return result; | 148 | return result; | ||
| 141 | } | 149 | } | ||
| 142 | 150 | | |||
| 143 | void addIncludes(QVector<const char*>* args, QVector<QByteArray>* otherArgs, | 151 | void addIncludes(QVector<const char*>* args, QVector<QByteArray>* otherArgs, | ||
| 144 | const Path::List& includes, const char* cliSwitch) | 152 | const Path::List& includes, const char* cliSwitch) | ||
| 145 | { | 153 | { | ||
| 146 | for (const Path& url : includes) { | 154 | 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