Address some lints

Summary:
Address 3 lints:
1. Use std::vector::at() instead of std::vector::operator[] in the unit test.
2. Add directive to ignore 'bugprone-use-after-move' lint in Task.cpp.
3. Avoid extra copy of shared_ptr in the split preload functor.

Reviewed By: amitkdutta, pranjalssh

Differential Revision: D54180033

fbshipit-source-id: 5d26ed28dd0f482a24ba949a49b34ebc0f00bb71

velox/exec/TableScan.cpp

@@ -321,12 +321,14 @@ void TableScan::checkPreload() {
         maxSplitPreloadPerDriver_;
     if (!splitPreloader_) {
       splitPreloader_ =
-          [executor, this](std::shared_ptr<connector::ConnectorSplit> split) {
+          [executor,
+           this](const std::shared_ptr<connector::ConnectorSplit>& split) {
             preload(split);
 
-            executor->add([taskHolder = operatorCtx_->task(), split]() mutable {
-              split->dataSource->prepare();
-              split.reset();
+            executor->add([taskHolder = operatorCtx_->task(),
+                           connectorSplit = split]() mutable {
+              connectorSplit->dataSource->prepare();
+              connectorSplit.reset();
             });
           };
     }

velox/exec/TableScan.h

@@ -99,7 +99,7 @@ class TableScan : public SourceOperator {
   // callback can schedule preloads on an executor. These preloads may
   // outlive the Task and therefore need to capture a shared_ptr to
   // it.
-  std::function<void(std::shared_ptr<connector::ConnectorSplit>)>
+  std::function<void(const std::shared_ptr<connector::ConnectorSplit>&)>
       splitPreloader_{nullptr};
 
   // Count of splits that started background preload.

velox/exec/Task.cpp

@@ -1171,6 +1171,8 @@ bool Task::addSplitWithSequence(
   }
 
   if (!isTaskRunning) {
+    // Safe because 'split' is moved away above only if 'isTaskRunning'.
+    // @lint-ignore CLANGTIDY bugprone-use-after-move
     addRemoteSplit(planNodeId, split);
   }
 
@@ -1195,6 +1197,7 @@ void Task::addSplit(const core::PlanNodeId& planNodeId, exec::Split&& split) {
 
   if (!isTaskRunning) {
     // Safe because 'split' is moved away above only if 'isTaskRunning'.
+    // @lint-ignore CLANGTIDY bugprone-use-after-move
     addRemoteSplit(planNodeId, split);
   }
 }
@@ -1365,8 +1368,7 @@ BlockingReason Task::getSplitOrFuture(
     exec::Split& split,
     ContinueFuture& future,
     int32_t maxPreloadSplits,
-    const std::function<void(std::shared_ptr<connector::ConnectorSplit>)>&
-        preload) {
+    const ConnectorSplitPreloadFunc& preload) {
   std::lock_guard<std::timed_mutex> l(mutex_);
   auto& splitsState = getPlanNodeSplitsStateLocked(planNodeId);
   return getSplitOrFutureLocked(
@@ -1384,8 +1386,7 @@ BlockingReason Task::getSplitOrFutureLocked(
     exec::Split& split,
     ContinueFuture& future,
     int32_t maxPreloadSplits,
-    const std::function<void(std::shared_ptr<connector::ConnectorSplit>)>&
-        preload) {
+    const ConnectorSplitPreloadFunc& preload) {
   if (splitsStore.splits.empty()) {
     if (splitsStore.noMoreSplits) {
       return BlockingReason::kNotBlocked;
@@ -1405,8 +1406,7 @@ exec::Split Task::getSplitLocked(
     bool forTableScan,
     SplitsStore& splitsStore,
     int32_t maxPreloadSplits,
-    const std::function<void(std::shared_ptr<connector::ConnectorSplit>)>&
-        preload) {
+    const ConnectorSplitPreloadFunc& preload) {
   int32_t readySplitIndex = -1;
   if (maxPreloadSplits) {
     for (auto i = 0; i < splitsStore.splits.size() && i < maxPreloadSplits;

velox/exec/Task.h

@@ -31,6 +31,10 @@ class OutputBufferManager;
 
 class HashJoinBridge;
 class NestedLoopJoinBridge;
+
+using ConnectorSplitPreloadFunc =
+    std::function<void(const std::shared_ptr<connector::ConnectorSplit>&)>;
+
 class Task : public std::enable_shared_from_this<Task> {
  public:
   /// Creates a task to execute a plan fragment, but doesn't start execution
@@ -360,8 +364,7 @@ class Task : public std::enable_shared_from_this<Task> {
       exec::Split& split,
       ContinueFuture& future,
       int32_t maxPreloadSplits = 0,
-      const std::function<void(std::shared_ptr<connector::ConnectorSplit>)>&
-          preload = nullptr);
+      const ConnectorSplitPreloadFunc& preload = nullptr);
 
   void splitFinished(bool fromTableScan, int64_t splitWeight);
 
@@ -791,18 +794,16 @@ class Task : public std::enable_shared_from_this<Task> {
       SplitsStore& splitsStore,
       exec::Split& split,
       ContinueFuture& future,
-      int32_t maxPreloadSplits = 0,
-      const std::function<void(std::shared_ptr<connector::ConnectorSplit>)>&
-          preload = nullptr);
+      int32_t maxPreloadSplits,
+      const ConnectorSplitPreloadFunc& preload);
 
   /// Returns next split from the store. The caller must ensure the store is not
   /// empty.
   exec::Split getSplitLocked(
       bool forTableScan,
       SplitsStore& splitsStore,
       int32_t maxPreloadSplits,
-      const std::function<void(std::shared_ptr<connector::ConnectorSplit>)>&
-          preload);
+      const ConnectorSplitPreloadFunc& preload);
 
   // Creates for the given split group and fills up the 'SplitGroupState'
   // structure, which stores inter-operator state (local exchange, bridges).

velox/exec/tests/TableScanTest.cpp

@@ -1381,7 +1381,7 @@ TEST_F(TableScanTest, tableScanSplitsAndWeights) {
   const auto filePaths = makeFilePaths(numSplits);
   auto vectors = makeVectors(numSplits, 100);
   for (auto i = 0; i < numSplits; i++) {
-    writeToFile(filePaths[i]->path, vectors[i]);
+    writeToFile(filePaths.at(i)->path, vectors.at(i));
   }
 
   // Set the table scan operators wait twice:
@@ -1462,7 +1462,7 @@ TEST_F(TableScanTest, tableScanSplitsAndWeights) {
   for (auto fileIndex = 0; fileIndex < numSplits; ++fileIndex) {
     const int64_t splitWeight = fileIndex * 10 + 1;
     totalSplitWeights += splitWeight;
-    auto split = makeHiveSplit(filePaths[fileIndex]->path, splitWeight);
+    auto split = makeHiveSplit(filePaths.at(fileIndex)->path, splitWeight);
     task->addSplit(scanNodeId, std::move(split));
   }
   task->noMoreSplits(scanNodeId);