[Libreoffice-commits] core.git: compilerplugins/clang
Stephan Bergmann
sbergman at redhat.com
Sun Dec 10 21:53:09 UTC 2017
compilerplugins/clang/compat.hxx | 14 ++
compilerplugins/clang/salcall.cxx | 187 ++++++++++++++++++++-------------
compilerplugins/clang/test/salcall.cxx | 19 +++
3 files changed, 148 insertions(+), 72 deletions(-)
New commits:
commit 456f943d2258164d35ec3522b1866302aa1f6e6b
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Sun Dec 10 19:52:57 2017 +0100
Fix isSalCallFunction further
...after a31267be1bb42e8a5f80a3b660bbf969eeb5b647 "Fix isSalCallFunction so it
also works on Windows", so that it actually does work on Windows.
Change-Id: I0218fb41b3e1000e2325967a18dfaafaa95fe415
Reviewed-on: https://gerrit.libreoffice.org/46193
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx
index 532ce462887b..60eab1251e28 100644
--- a/compilerplugins/clang/compat.hxx
+++ b/compilerplugins/clang/compat.hxx
@@ -206,6 +206,20 @@ inline void addPPCallbacks(
#endif
}
+inline bool isPointWithin(
+ clang::SourceManager const & SM, clang::SourceLocation Location, clang::SourceLocation Start,
+ clang::SourceLocation End)
+{
+#if CLANG_VERSION >= 60000
+ return SM.isPointWithin(Location, Start, End);
+#else
+ return
+ Location == Start || Location == End
+ || (SM.isBeforeInTranslationUnit(Start, Location)
+ && SM.isBeforeInTranslationUnit(Location, End));
+#endif
+}
+
inline bool isMacroArgExpansion(
clang::CompilerInstance& compiler, clang::SourceLocation location,
clang::SourceLocation * startLocation)
diff --git a/compilerplugins/clang/salcall.cxx b/compilerplugins/clang/salcall.cxx
index 0f73e15adf66..3b291cb79f56 100644
--- a/compilerplugins/clang/salcall.cxx
+++ b/compilerplugins/clang/salcall.cxx
@@ -9,8 +9,10 @@
#include "plugin.hxx"
#include "check.hxx"
+#include "compat.hxx"
#include <algorithm>
+#include <cassert>
#include <set>
#include <utility>
#include <vector>
@@ -333,6 +335,8 @@ bool SalCall::VisitFunctionDecl(FunctionDecl const* decl)
return true;
}
+//TODO: This doesn't handle all possible cases of macro usage (and possibly never will be able to),
+// just what is encountered in practice:
bool SalCall::isSalCallFunction(FunctionDecl const* functionDecl, SourceLocation* pLoc)
{
assert(!functionDecl->isTemplateInstantiation());
@@ -358,18 +362,10 @@ bool SalCall::isSalCallFunction(FunctionDecl const* functionDecl, SourceLocation
}
SourceManager& SM = compiler.getSourceManager();
-
- // Stop searching for "SAL_CALL" at the start of the function declaration's name (for qualified
- // names this will point after the qualifiers, but needlessly including those in the search
- // should be harmless):
- SourceLocation endLoc = functionDecl->getNameInfo().getLocStart();
- while (endLoc.isMacroID() && SM.isAtStartOfImmediateMacroExpansion(endLoc))
- {
- endLoc = SM.getImmediateMacroCallerLoc(endLoc);
- }
- endLoc = SM.getSpellingLoc(endLoc);
+ std::vector<SourceRange> ranges;
SourceLocation startLoc;
+ SourceLocation endLoc;
bool noReturnType = isa<CXXConstructorDecl>(functionDecl)
|| isa<CXXDestructorDecl>(functionDecl)
|| isa<CXXConversionDecl>(functionDecl);
@@ -410,37 +406,81 @@ bool SalCall::isSalCallFunction(FunctionDecl const* functionDecl, SourceLocation
return false;
}
startLoc = FTL.getReturnLoc().getEndLoc();
- auto const slEnd = Lexer::getLocForEndOfToken(startLoc, 0, SM, compiler.getLangOpts());
- if (slEnd.isValid()) // i.e., startLoc either non-macro, or at end of macro
+ while (SM.isMacroArgExpansion(startLoc, &startLoc))
{
- startLoc = slEnd;
}
- else // otherwise, resolve into macro text
+
+ // Stop searching for "SAL_CALL" at the start of the function declaration's name (for
+ // qualified names this will point after the qualifiers, but needlessly including those in
+ // the search should be harmless---modulo issues with using "SAL_CALL" as the name of a
+ // function-like macro parameter as discussed below):
+ endLoc = functionDecl->getNameInfo().getLocStart();
+ while (SM.isMacroArgExpansion(endLoc, &endLoc))
{
- startLoc = Lexer::getLocForEndOfToken(SM.getSpellingLoc(startLoc), 0, SM,
- compiler.getLangOpts());
}
- while (startLoc.isMacroID() && SM.isAtEndOfImmediateMacroExpansion(startLoc, &startLoc))
+ while (endLoc.isMacroID() && SM.isAtStartOfImmediateMacroExpansion(endLoc))
{
+ endLoc = SM.getImmediateMacroCallerLoc(endLoc);
}
- startLoc = SM.getSpellingLoc(startLoc);
+ endLoc = SM.getSpellingLoc(endLoc);
- if (startLoc.isValid() && endLoc.isValid() && startLoc != endLoc
- && !SM.isBeforeInTranslationUnit(startLoc, endLoc))
+ auto const slEnd = Lexer::getLocForEndOfToken(startLoc, 0, SM, compiler.getLangOpts());
+ if (slEnd.isValid())
{
- // Happens for uses of trailing return type (in which case starting instead at the start
- // of the function declaration should be fine), but also for cases like
- //
- // void (*f())();
- //
- // where the function name is within the function type (TODO: in which case starting at
- // the start can erroneously pick up the "SAL_CALL" from the returned pointer-to-
- // function type in cases like
- //
- // void SAL_CALL (*f())();
- //
- // that are hopefully rare):
- startAfterReturnType = false;
+ // startLoc is either non-macro, or at end of macro; one source range from startLoc to
+ // endLoc:
+ startLoc = slEnd;
+ while (startLoc.isMacroID() && SM.isAtEndOfImmediateMacroExpansion(startLoc, &startLoc))
+ {
+ }
+ startLoc = SM.getSpellingLoc(startLoc);
+
+ if (startLoc.isValid() && endLoc.isValid() && startLoc != endLoc
+ && !SM.isBeforeInTranslationUnit(startLoc, endLoc))
+ {
+ // Happens for uses of trailing return type (in which case starting instead at the
+ // start of the function declaration should be fine), but also for cases like
+ //
+ // void (*f())();
+ //
+ // where the function name is within the function type (TODO: in which case starting
+ // at the start can erroneously pick up the "SAL_CALL" from the returned pointer-to-
+ // function type in cases like
+ //
+ // void SAL_CALL (*f())();
+ //
+ // that are hopefully rare):
+ startAfterReturnType = false;
+ }
+ }
+ else
+ {
+ // startLoc is within a macro body; two source ranges, first is the remainder of the
+ // corresponding macro definition's replacement text, second is from after the macro
+ // invocation to endLoc, unless endLoc is already in the first range:
+ //TODO: If the macro is a function-like macro with a parameter named "SAL_CALL", uses of
+ // that parameter in the remainder of the replacement text will be false positives.
+ assert(SM.isMacroBodyExpansion(startLoc));
+ auto const startLoc2 = SM.getImmediateExpansionRange(startLoc).second;
+ auto const MI
+ = compiler.getPreprocessor()
+ .getMacroDefinitionAtLoc(
+ &compiler.getASTContext().Idents.get(
+ Lexer::getImmediateMacroName(startLoc, SM, compiler.getLangOpts())),
+ SM.getSpellingLoc(startLoc))
+ .getMacroInfo();
+ assert(MI != nullptr);
+ auto endLoc1 = MI->getDefinitionEndLoc();
+ assert(endLoc1.isFileID());
+ endLoc1 = Lexer::getLocForEndOfToken(endLoc1, 0, SM, compiler.getLangOpts());
+ startLoc = Lexer::getLocForEndOfToken(SM.getSpellingLoc(startLoc), 0, SM,
+ compiler.getLangOpts());
+ if (!compat::isPointWithin(SM, endLoc, startLoc, endLoc1))
+ {
+ ranges.emplace_back(startLoc, endLoc1);
+ startLoc = Lexer::getLocForEndOfToken(SM.getSpellingLoc(startLoc2), 0, SM,
+ compiler.getLangOpts());
+ }
}
}
if (!startAfterReturnType)
@@ -491,60 +531,63 @@ bool SalCall::isSalCallFunction(FunctionDecl const* functionDecl, SourceLocation
}
}
#endif
- }
- if (startLoc.isInvalid() || endLoc.isInvalid())
- {
- if (isDebugMode())
+ // Stop searching for "SAL_CALL" at the start of the function declaration's name (for
+ // qualified names this will point after the qualifiers, but needlessly including those in
+ // the search should be harmless):
+ endLoc = functionDecl->getNameInfo().getLocStart();
+ while (endLoc.isMacroID() && SM.isAtStartOfImmediateMacroExpansion(endLoc))
{
- report(DiagnosticsEngine::Fatal, "TODO: unexpected failure #2, needs investigation",
- functionDecl->getLocation())
- << functionDecl->getSourceRange();
+ endLoc = SM.getImmediateMacroCallerLoc(endLoc);
}
- return false;
+ endLoc = SM.getSpellingLoc(endLoc);
}
- if (isDebugMode() && startLoc != endLoc && !SM.isBeforeInTranslationUnit(startLoc, endLoc))
- {
- report(DiagnosticsEngine::Fatal, "TODO: unexpected failure #3, needs investigation",
- functionDecl->getLocation())
- << functionDecl->getSourceRange();
- }
-
- SourceLocation found;
+ ranges.emplace_back(startLoc, endLoc);
- while (SM.isBeforeInTranslationUnit(startLoc, endLoc))
+ for (auto const range : ranges)
{
- unsigned n = Lexer::MeasureTokenLength(startLoc, SM, compiler.getLangOpts());
- auto s = StringRef(compiler.getSourceManager().getCharacterData(startLoc), n);
- while (s.startswith("\\\n"))
+ if (range.isInvalid())
{
- s = s.drop_front(2);
- while (!s.empty()
- && (s.front() == ' ' || s.front() == '\t' || s.front() == '\n'
- || s.front() == '\v' || s.front() == '\f'))
+ if (isDebugMode())
{
- s = s.drop_front(1);
+ report(DiagnosticsEngine::Fatal, "TODO: unexpected failure #2, needs investigation",
+ functionDecl->getLocation())
+ << functionDecl->getSourceRange();
}
+ return false;
}
- if (s == "SAL_CALL")
+ if (isDebugMode() && range.getBegin() != range.getEnd()
+ && !SM.isBeforeInTranslationUnit(range.getBegin(), range.getEnd()))
{
- found = startLoc;
- break;
+ report(DiagnosticsEngine::Fatal, "TODO: unexpected failure #3, needs investigation",
+ functionDecl->getLocation())
+ << functionDecl->getSourceRange();
}
- if (startLoc == endLoc)
+
+ for (auto loc = range.getBegin(); SM.isBeforeInTranslationUnit(loc, range.getEnd());)
{
- break;
+ unsigned n = Lexer::MeasureTokenLength(loc, SM, compiler.getLangOpts());
+ auto s = StringRef(compiler.getSourceManager().getCharacterData(loc), n);
+ while (s.startswith("\\\n"))
+ {
+ s = s.drop_front(2);
+ while (!s.empty()
+ && (s.front() == ' ' || s.front() == '\t' || s.front() == '\n'
+ || s.front() == '\v' || s.front() == '\f'))
+ {
+ s = s.drop_front(1);
+ }
+ }
+ if (s == "SAL_CALL")
+ {
+ if (pLoc)
+ *pLoc = loc;
+ return true;
+ }
+ loc = loc.getLocWithOffset(std::max<unsigned>(n, 1));
}
- startLoc = startLoc.getLocWithOffset(std::max<unsigned>(n, 1));
}
-
- if (found.isInvalid())
- return false;
-
- if (pLoc)
- *pLoc = found;
-
- return true;
+ return false;
}
bool SalCall::rewrite(SourceLocation locBegin)
diff --git a/compilerplugins/clang/test/salcall.cxx b/compilerplugins/clang/test/salcall.cxx
index e3ed95b106ce..736bb27a3b6a 100644
--- a/compilerplugins/clang/test/salcall.cxx
+++ b/compilerplugins/clang/test/salcall.cxx
@@ -117,6 +117,25 @@ class Class8_3 : public Class8_1, public Class8_2
virtual ~Class8_3();
};
+#define M1(m) void m
+class Class9
+{
+ M1(method1)(); // expected-error {{SAL_CALL inconsistency [loplugin:salcall]}}
+ void method2(); // expected-error {{SAL_CALL inconsistency [loplugin:salcall]}}
+};
+void SAL_CALL Class9::method1() {} // expected-note {{SAL_CALL inconsistency [loplugin:salcall]}}
+#define M2(T) T SAL_CALL
+M2(void) Class9::method2() {} // expected-note {{SAL_CALL inconsistency [loplugin:salcall]}}
+
+#if 0 // see TODO in SalCall::isSalCallFunction
+class Class10
+{
+ void method1();
+};
+#define M3(T, SAL_CALL) T SAL_CALL::
+M3(void, Class10) method1() {} // false "SAL_CALL inconsistency"
+#endif
+
#if 0 //TODO
template<typename> struct S {
virtual ~S();
More information about the Libreoffice-commits
mailing list