Remove non-const Sequence::begin()/end() in internal code

... to avoid hidden cost of multiple COW checks, because they
call getArray() internally.
This obsoletes [loplugin:sequenceloop].

Also rename toNonConstRange to asNonConstRange, to reflect that
the result is a view of the sequence, not an independent object.

TODO: also drop non-const operator[], but introduce operator[]
in SequenceRange.

Change-Id: Idd5fd7a3400fe65274d2a6343025e2ef8911635d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123518
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

3 files changed, 1 insertions, 132 deletions

diff --git a/compilerplugins/clang/sequenceloop.cxx b/compilerplugins/clang/sequenceloop.cxx
deleted file mode 100644
index 8c931d70d5d1..000000000000
--- a/compilerplugins/clang/sequenceloop.cxx
+++ /dev/null
@@ -1,83 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
- */
-#ifndef LO_CLANG_SHARED_PLUGINS
-
-#include <cassert>
-#include <string>
-#include <iostream>
-#include <fstream>
-#include <set>
-
-#include <clang/AST/CXXInheritance.h>
-#include "plugin.hxx"
-#include "check.hxx"
-
-/**
-  When used in "for" loops, css::uno::Sequence objects tend to end up calling the non-const begin()/end(),
-  which is considerably more expensive than the const variants because it forces a local copy
-  of the internal ref-counted impl object.
-*/
-
-namespace
-{
-class SequenceLoop : public loplugin::FilteringPlugin<SequenceLoop>
-{
-public:
-    explicit SequenceLoop(loplugin::InstantiationData const& data)
-        : FilteringPlugin(data)
-    {
-    }
-
-    virtual void run() override
-    {
-        if (preRun())
-            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
-    }
-
-    bool VisitCXXForRangeStmt(CXXForRangeStmt const*);
-};
-
-bool SequenceLoop::VisitCXXForRangeStmt(CXXForRangeStmt const* forStmt)
-{
-    if (ignoreLocation(forStmt))
-        return true;
-
-    auto tc = loplugin::TypeCheck(forStmt->getRangeInit()->getType());
-    if (tc.Const())
-        return true;
-    if (!tc.Class("Sequence")
-             .Namespace("uno")
-             .Namespace("star")
-             .Namespace("sun")
-             .Namespace("com")
-             .GlobalNamespace()
-        // B2DPolyPolygon is similar in that accessing the non-const begin()/end() methods
-        // might trigger unnecessary copying
-        && !tc.Class("B2DPolyPolygon").Namespace("basegfx").GlobalNamespace())
-        return true;
-    const VarDecl* varDecl = forStmt->getLoopVariable();
-    auto tc2 = loplugin::TypeCheck(varDecl->getType());
-    if (!tc2.LvalueReference().Const())
-        return true;
-
-    report(DiagnosticsEngine::Warning,
-           ("use std::as_const, or otherwise make the for-range-initializer expression const, to"
-            " avoid creating a copy of the Sequence"),
-           compat::getBeginLoc(forStmt->getRangeInit()))
-        << forStmt->getSourceRange();
-    return true;
-}
-
-loplugin::Plugin::Registration<SequenceLoop> sequenceloop("sequenceloop");
-
-} // namespace
-
-#endif // LO_CLANG_SHARED_PLUGINS
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/constvars.cxx b/compilerplugins/clang/test/constvars.cxx
index dc3c1ecb9c6b..88df50f8e199 100644
--- a/compilerplugins/clang/test/constvars.cxx
+++ b/compilerplugins/clang/test/constvars.cxx
@@ -12,7 +12,6 @@
 #else
 
 #include <com/sun/star/uno/Any.hxx>
-#include <com/sun/star/uno/Sequence.hxx>
 #include <com/sun/star/uno/XInterface.hpp>
 #include <map>
 #include <list>
@@ -79,20 +78,8 @@ void foo(std::list<Struct1*> aList)
 }
 };
 
-namespace test6
-{
-void foo(css::uno::Sequence<css::uno::Reference<css::uno::XInterface>>& aSeq)
-{
-    // expected-error@+1 {{var can be const [loplugin:constvars]}}
-    for (css::uno::Reference<css::uno::XInterface>& x : aSeq)
-    {
-        x.get();
-    }
-}
-};
-
 // no warning expected
-namespace test7
+namespace test6
 {
 void foo(std::vector<std::vector<int>> aVecVec)
 {
diff --git a/compilerplugins/clang/test/sequenceloop.cxx b/compilerplugins/clang/test/sequenceloop.cxx
deleted file mode 100644
index e124fda27093..000000000000
--- a/compilerplugins/clang/test/sequenceloop.cxx
+++ /dev/null
@@ -1,35 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
- */
-
-#include <com/sun/star/uno/Sequence.hxx>
-#include <com/sun/star/uno/XInterface.hpp>
-#include <utility>
-
-namespace test1
-{
-void foo(css::uno::Sequence<css::uno::Reference<css::uno::XInterface>>& aSeq)
-{
-    // expected-error@+1 {{use std::as_const, or otherwise make the for-range-initializer expression const, to avoid creating a copy of the Sequence [loplugin:sequenceloop]}}
-    for (const auto& x : aSeq)
-        x.get();
-    // no warning expected
-    for (auto& x : aSeq)
-        x.get();
-    for (const auto& x : std::as_const(aSeq))
-        x.get();
-}
-// no warning expected
-void foo2(const css::uno::Sequence<css::uno::Reference<css::uno::XInterface>>& aSeq)
-{
-    for (const auto& x : aSeq)
-        x.get();
-}
-};
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */