loplugin:oncevar for value-dependent constant expressions

(Where the change to basic/source/comp/codegen.cxx reveals that
loplugin:loopvartoosmall also needs the Clang < 3.9 workaround from
33ee8e61292af05627f5f72ea2f93ad80e715e46 "Work around bug in Clang 3.8".)

Change-Id: I9f23b9648bc11ca4136a0fbdd332570ba70ee77c
Reviewed-on: https://gerrit.libreoffice.org/39667
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

4 files changed, 65 insertions, 18 deletions

diff --git a/basic/source/comp/codegen.cxx b/basic/source/comp/codegen.cxx
index 384344e2f9b1..64791af66671 100644
--- a/basic/source/comp/codegen.cxx
+++ b/basic/source/comp/codegen.cxx
@@ -22,6 +22,7 @@
 #include "image.hxx"
 #include "codegen.hxx"
 #include "parser.hxx"
+#include <cstddef>
 #include <limits>
 #include <algorithm>
 #include <osl/diagnose.h>
@@ -408,9 +409,8 @@ private:
     const sal_uInt8* m_pCode;
     static T readParam( sal_uInt8 const *& pCode )
     {
-        short nBytes = sizeof( T );
         T nOp1=0;
-        for ( int i=0; i<nBytes; ++i )
+        for ( std::size_t i=0; i<sizeof( T ); ++i )
             nOp1 |= *pCode++ << ( i * 8);
         return nOp1;
     }
diff --git a/compilerplugins/clang/loopvartoosmall.cxx b/compilerplugins/clang/loopvartoosmall.cxx
index f9c25eb4d8a9..66eda67167e2 100644
--- a/compilerplugins/clang/loopvartoosmall.cxx
+++ b/compilerplugins/clang/loopvartoosmall.cxx
@@ -149,7 +149,16 @@ void LoopVarTooSmall::checkSubExpr(Expr const * expr, bool positive) {
         return;
     unsigned qt2BitWidth;
     llvm::APSInt aIntResult;
-    if (binOpRHS->EvaluateAsInt(aIntResult, compiler.getASTContext())) {
+    // Work around missing Clang 3.9 fix <https://reviews.llvm.org/rL271762>
+    // "Sema: do not attempt to sizeof a dependent type", causing Clang 3.8 to
+    // crash during EvaluateAsInt() on expressions of the form
+    //
+    //   sizeof (T)
+    //
+    // with dependent type T:
+    if (!binOpRHS->isValueDependent()
+        && binOpRHS->EvaluateAsInt(aIntResult, compiler.getASTContext()))
+    {
         if (less && aIntResult.isStrictlyPositive()) {
             --aIntResult;
         }
diff --git a/compilerplugins/clang/oncevar.cxx b/compilerplugins/clang/oncevar.cxx
index e7b55a09421f..7fae0e17f776 100644
--- a/compilerplugins/clang/oncevar.cxx
+++ b/compilerplugins/clang/oncevar.cxx
@@ -7,6 +7,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+#include <cassert>
 #include <string>
 #include <iostream>
 #include <unordered_map>
@@ -15,6 +16,7 @@
 #include "plugin.hxx"
 #include "check.hxx"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/StmtVisitor.h"
 
 // Original idea from tml.
 // Look for variables that are (a) initialised from zero or one constants. (b) only used in one spot.
@@ -27,6 +29,43 @@ bool startsWith(const std::string& rStr, const char* pSubStr) {
     return rStr.compare(0, strlen(pSubStr), pSubStr) == 0;
 }
 
+class ConstantValueDependentExpressionVisitor:
+    public ConstStmtVisitor<ConstantValueDependentExpressionVisitor, bool>
+{
+    ASTContext const & context_;
+
+public:
+    ConstantValueDependentExpressionVisitor(ASTContext const & context):
+        context_(context) {}
+
+    bool Visit(Stmt const * stmt) {
+        assert(isa<Expr>(stmt));
+        auto const expr = cast<Expr>(stmt);
+        if (!expr->isValueDependent()) {
+            return expr->isEvaluatable(context_);
+        }
+        return ConstStmtVisitor::Visit(stmt);
+    }
+
+    bool VisitParenExpr(ParenExpr const * expr)
+    { return Visit(expr->getSubExpr()); }
+
+    bool VisitCastExpr(CastExpr const * expr) {
+        return Visit(expr->getSubExpr());
+    }
+
+    bool VisitUnaryOperator(UnaryOperator const * expr)
+    { return Visit(expr->getSubExpr()); }
+
+    bool VisitBinaryOperator(BinaryOperator const * expr) {
+        return Visit(expr->getLHS()) && Visit(expr->getRHS());
+    }
+
+    bool VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr const *) {
+        return true;
+    }
+};
+
 class OnceVar:
     public RecursiveASTVisitor<OnceVar>, public loplugin::Plugin
 {
@@ -99,6 +138,11 @@ private:
     std::unordered_set<VarDecl const *> maVarDeclToIgnoreSet;
     std::unordered_map<VarDecl const *, int> maVarUsesMap;
     std::unordered_map<VarDecl const *, SourceRange> maVarUseSourceRangeMap;
+
+    bool isConstantValueDependentExpression(Expr const * expr) {
+        return ConstantValueDependentExpressionVisitor(compiler.getASTContext())
+            .Visit(expr);
+    }
 };
 
 bool OnceVar::VisitVarDecl( const VarDecl* varDecl )
@@ -164,21 +208,10 @@ bool OnceVar::VisitVarDecl( const VarDecl* varDecl )
     }
     if (!foundStringLiteral) {
         auto const init = varDecl->getInit();
-#if CLANG_VERSION < 30900
-        // Work around missing Clang 3.9 fix <https://reviews.llvm.org/rL271762>
-        // "Sema: do not attempt to sizeof a dependent type" (while an
-        // initializer expression of the form
-        //
-        //   sizeof (T)
-        //
-        // with dependent type T /is/ constant, keep consistent here with the
-        // (arguably broken) behavior of isConstantInitializer returning false
-        // in Clang >= 3.9):
-        if (init->isValueDependent()) {
-            return true;
-        }
-#endif
-        if (!init->isConstantInitializer(compiler.getASTContext(), false/*ForRef*/))
+        if (!(init->isValueDependent()
+              ? isConstantValueDependentExpression(init)
+              : init->isConstantInitializer(
+                  compiler.getASTContext(), false/*ForRef*/)))
         {
             return true;
         }
diff --git a/compilerplugins/clang/test/oncevar.cxx b/compilerplugins/clang/test/oncevar.cxx
index 32ff42d4479d..8c7b8d2a9e1d 100644
--- a/compilerplugins/clang/test/oncevar.cxx
+++ b/compilerplugins/clang/test/oncevar.cxx
@@ -18,6 +18,11 @@ void call_value(OUString);
 void call_const_ref(OUString const &);
 void call_ref(OUString &);
 
+template<typename T> void f() {
+    int i = sizeof (T) + 1; // expected-error {{var used only once, should be inlined or declared const [loplugin:oncevar]}}
+    call_value(i); // expected-note {{used here [loplugin:oncevar]}}
+}
+
 int main() {
 /* TODO
     int i;