Commit 902bc682 authored by Igor Kushnir's avatar Igor Kushnir
Browse files

Fix Visitor::setDeclData(CXCursor, MacroDefinition*)

The following mistakes were present in the old code:
1) a tab or a comment after a macro identifier was not considered;
2) a space was considered mandatory after ')'.

In practice, this lead to bugs specified in the table below. The first
column is the parsed macro code. The second and third columns - the text
after "Body: " in the macro's tooltip before and at this commit
respectively.
 C++ code                   Body before     Body at
 #define m(x)x              m(x)x           x
 #define m(x)long x         ong x           long x
 #define m(x)/*A*/x         m(x)/*A*/x      /*A*/x
 #define m/*o*/long         m/*o*/long      /*o*/long
 #define m/*(x)*/ long      / long          /*(x)*/ long
parent b5b16f6a
......@@ -39,6 +39,7 @@
#include <QVarLengthArray>
#include <algorithm>
#include <unordered_map>
#include <typeinfo>
......@@ -1049,36 +1050,55 @@ void Visitor::setDeclData(CXCursor cursor, MacroDefinition* decl) const
// And no way to get the actual definition text range
// Should be quite easy to expose that in libclang, though
// Let' still get some basic support for this and parse on our own, it's not that difficult
const QString contents = ClangUtils::getRawContents(unit, range);
const int firstOpeningParen = contents.indexOf(QLatin1Char('('));
const int firstWhitespace = contents.indexOf(QLatin1Char(' '));
const bool isFunctionLike = (firstOpeningParen != -1) && (firstOpeningParen < firstWhitespace);
decl->setFunctionLike(isFunctionLike);
// now extract the actual definition text
int start = -1;
if (isFunctionLike) {
const int closingParen = findClose(contents, firstOpeningParen);
if (closingParen != -1) {
start = closingParen + 2; // + ')' + ' '
// extract macro function parameters
const auto parameters = QStringView{contents}.mid(firstOpeningParen, closingParen - firstOpeningParen + 1);
ParamIterator paramIt(u"():", parameters, 0);
while (paramIt) {
decl->addParameter(IndexedString(*paramIt));
++paramIt;
}
}
} else {
start = firstWhitespace + 1; // + ' '
// Macro definition strings get '\n' replaced with "<br/>", then are rendered as HTML, which ignores whitespace.
// Newline characters are escaped in macro definiton C++ code. ClangUtils::getRawContents() preserves the escape
// characters: its return value contains "\\\n". ClangUtils::getRawContents() removes whitespace, including the
// escaped newline characters, at the end of the definition string.
// Trim definition string views before passing them to MacroDefinition::setDefinition() to facilitate testing.
// This trimming never strips newline characters in practice (shouldn't have been a problem even if it did).
const auto setDefinition = [decl](QStringView definition) {
decl->setDefinition(IndexedString{definition.trimmed()});
};
const QString rawContentsString = ClangUtils::getRawContents(unit, range);
// Use a QStringView contents, because it works as fast as or faster than a QString in the code below.
const QStringView contents(rawContentsString);
const auto isPartOfIdentifier = [](QChar c) {
return c.isLetterOrNumber() || c == QLatin1Char('_');
};
const int posAfterMacroId =
std::find_if_not(contents.cbegin(), contents.cend(), isPartOfIdentifier) - contents.cbegin();
if (posAfterMacroId == contents.size() || contents[posAfterMacroId] != QLatin1Char{'('}) {
// '(', a space, a tab or '/' (a comment) usually follows a macro identifier.
// Compilers consider a macro function-like only if '(' immediately follows its identifier.
decl->setFunctionLike(false);
setDefinition(contents.mid(posAfterMacroId));
return;
}
if (start == -1) {
decl->setFunctionLike(true);
const auto openingParen = posAfterMacroId;
const auto closingParen = findClose(contents, openingParen);
if (closingParen == -1) {
// unlikely: invalid macro definition, insert the complete #define statement
decl->setDefinition(IndexedString(QLatin1String("#define ") + contents));
} else if (start < contents.size()) {
decl->setDefinition(IndexedString(contents.mid(start)));
} // else: macro has no body => leave the definition text empty
const QString definition = QLatin1String("#define ") + contents;
setDefinition(definition);
return;
}
setDefinition(contents.mid(closingParen + 1));
// extract macro function parameters
const auto parameters = contents.mid(openingParen, closingParen - openingParen + 1); // include both '(' and ')'
ParamIterator paramIt(u"():", parameters, 0);
while (paramIt) {
decl->addParameter(IndexedString(*paramIt));
++paramIt;
}
}
template<CXCursorKind CK>
......
......@@ -256,17 +256,29 @@ void TestDUChain::testMacroDefinition_data()
<< QString{"#define " + code} << definition << isFunctionLike << parameters;
};
addTest("macro", "");
addTest("m1 1", "1");
addTest("m int x", "int x");
addTest("m (u + v)", "(u + v)");
addTest("m\tn", "n");
addTest("m/*o*/long", "/*o*/long");
addTest("m/*(x)*/ long", "/*(x)*/ long");
addTest("mac(x)", "", true, {"x"});
addTest("VARG_1(...)", "", true, {"..."});
addTest("mac(x) x", "x", true, {"x"});
addTest("mc(u)\tu *2 \t/ Limit", "u *2 \t/ Limit", true, {"u"});
addTest("m(x)x", "x", true, {"x"});
addTest("m(x)long x", "long x", true, {"x"});
addTest("m(x)/*A*/x", "/*A*/x", true, {"x"});
addTest("XYZ_(...) __VA_ARGS__", "__VA_ARGS__", true, {"..."});
addTest("macro(may,be)", "", true, {"may", "be"});
addTest("m(x, \ty)\tx", "x", true, {"x", "y"});
addTest("m(x, y) x / y", "x / y", true, {"x", "y"});
addTest("M_N(X, ...) f(X __VA_OPT__(,) __VA_ARGS__)", "f(X __VA_OPT__(,) __VA_ARGS__)", true, {"X", "..."});
addTest("MM\t\\\n\t471", "\\\n\t471");
}
void TestDUChain::testInclude()
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment