mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
cool! but needs some work :)
INLINE COMMENTS
> ibuildsystemmanager.h:137
> + */
> + virtual Path findCompiler(KDevelop::ProjectTargetItem* p) const { Q_UNUSED(p); return {}; };
> };
make it pure virtual like the rest of this interface
rename to `compiler`
> cmakemanager.cpp:1002
> + if (targetInfo.sources.isEmpty()) {
> + qDebug() << "could not find target" << item->text();
> + return {};
category-based logging, also below
> cmakemanager.cpp:1009
> + if (lang.isEmpty()) {
> + qDebug() << "no lang for..." << item << item->text() << info.defines << targetInfo.sources.constFirst();
> + return {};
remove ...
> cmakemanager.cpp:1012
> + }
> + const QString var = QStringLiteral("CMAKE_") + lang + QStringLiteral("_COMPILER");
> + const auto ret = CMake::readCacheValues(KDevelop::Path(buildDirectory(item), QStringLiteral("CMakeCache.txt")), {var});
use QLatin1String since you are concatenating
> compilerprovider.cpp:39
> #include <QDir>
> +#include <iprojectcontroller.h>
> +#include <interfaces/ibuildsystemmanager.h>
move up, add interfaces/
> compilerprovider.cpp:157
> + qCDebug(DEFINESANDINCLUDES) << "found compiler" << path;
> + if (!path.isEmpty() || !QDir::isAbsolutePath(path.toLocalFile()))
> + continue;
this isAbsolutePath check - shouldn't it be an assertion? the BSM should sanitize and produce absolute paths, I'd say (like, based on the build/source dir)
> compilerprovider.cpp:162
> + bool alreadyPresent = false;
> + for ( const CompilerPointer& compiler : m_compilers ) {
> + if (compiler->path() == pathString) {
`for (const auto& compiler : m_compilers)`
or better yet:
if (std::any_of(m_compilers.begin(), m_compilers.end(),
[pathString](const auto& compiler) { compiler->path() == pathString; })
{
continue;
}
> compilerprovider.cpp:171
> + //we need to search, sdk compiler names are weird: arm-linux-androideabi-g++
> + int factory = path.lastPathSegment().contains(QLatin1String("clang")) ? 1 : 0;
> + auto compiler = m_factories[factory]->createCompiler(path.lastPathSegment(), pathString);
this looks weird to me - this shouldn't be hardcoded like this, no? it should go through all factories and find a match. I.e. on windows we may need to use the msvc factory?
REPOSITORY
R32 KDevelop
REVISION DETAIL
https://phabricator.kde.org/D11136
To: apol, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180308/c72ea186/attachment.html>