Use .at() to avoid UB in tutorial code. Fix a bug in "correct" implem…

…entation.

docs/tutorial.cpp

@@ -12,8 +12,8 @@ double add(double first, double second) {
 //           contain [4,0,1,3,3]
 void slideRight(std::vector<int> &v) {
   // NOTE - This implementation contains a bug!
-  for(int i = 1; i < static_cast<int>(v.size()); ++i) {
-    v[i] = v[i-1];
+  for (int i = 1; i < static_cast<int>(v.size()); ++i) {
+    v.at(i) = v.at(i-1);
   }
 }
 
@@ -25,11 +25,11 @@ void flip(std::vector<int> &v) {
   // NOTE - This implementation contains a bug!
   int left = 0;
   int right = static_cast<int>(v.size()) - 1;
-  while(left != right) {
-    int temp = v[left];
-    v[left] = v[right];
-    v[right] = temp;
+  while (left != right) {
+    int temp = v.at(left);
+    v.at(left) = v.at(right);
+    v.at(right) = temp;
     ++left;
     --right;
   }
-}
+}

metatest-common.mk

@@ -0,0 +1,62 @@
+# The intent is for this makefile to provide common functionality that can be
+# included in a specific makefile for each lab/project that provides additional
+# targets.
+
+# Compiler and flags. Need to be provided here as the metatest makefiles
+# are not always used via recursive calls to make from the AG makefile.
+export CXX ?= g++
+export CXXFLAGS ?= --std=c++17 -Wall -pedantic -g -Wno-sign-compare -Wno-comment
+
+# Compiler and flags. Need to be provided here as the metatest makefiles
+# are not always used via recursive calls to make from the AG makefile.
+# Note that we expliclty set LD here so that it doesn't default to ld rather than g++.
+export LD := $(CXX)
+export LDFLAGS ?= -Wall
+
+# Set student_test at the command line when running the validity_check
+# target. If this value is not set, all the student tests will be run.  Set
+# student_test and bug_name at the command line when running the bug_check
+# target.  If student_test is not set, all the student tests will be run.
+student_test =
+
+# Test student unit tests against correct instructor solution.  This tests for
+# the absence of false positives, which we sometimes call a validity check.
+# Test student unit tests against buggy implementations, AKA true positives.
+# The recipe will only succeed if ALL buggy implemenations are caught by at
+# least one of the student unit tests.
+metatest: validity_check verify_all_bugs_caught
+
+# This intentionally has no dependencies
+get_test_names:
+	-@test -f $(SUITE_NAME)_validity_check.exe && ./$(SUITE_NAME)_validity_check.exe --show_test_names
+
+validity_check: $(SUITE_NAME)_validity_check.exe
+	@./$(SUITE_NAME)_validity_check.exe $(student_test)
+	@echo "****************************************"
+	@echo "VALIDITY CHECK PASS ($(SUITE_NAME)): student unit tests pass on instructor solution"
+
+bug_check_%: $(SUITE_NAME)_bug_check_%.exe
+	./$< $(student_test)
+
+verify_all_bugs_caught: $(patsubst %, verify_caught_%, $(ALL_BUGS))
+	@echo "****************************************"
+	@echo "BUG CATCHING PASS ($(SUITE_NAME)): student unit tests caught ALL instructor bugs"
+
+verify_caught_%: $(SUITE_NAME)_bug_check_%.exe
+	! ./$< $(student_test)
+
+.SECONDARY:
+
+.PHONY: clean metatest get_test_names validity_check bug_check verify_all_bugs_caught
+
+# Compile one object
+%.o: %.cpp
+	$(CXX) $(CXXFLAGS) $< -c -o $@
+
+# Verify variables set by metatest-*.mk
+ifndef SUITE_NAME
+  $(error SUITE_NAME is not set)
+endif
+ifndef ALL_BUGS
+  $(error ALL_BUGS is not set)
+endif

tutorial_buggy_impls.cpp

@@ -36,47 +36,43 @@ using namespace std;
 // Bug Description: The implementation slides left instead of right.
 
 void slideRight(std::vector<int> &v) {
-  int *arr = v.data();
-  int n = static_cast<int>(v.size());
-  int first = *arr;
-  for(int *ptr = arr+1; ptr < arr + n; ++ptr) {
-    *(ptr-1) = *ptr;
+  if (v.empty()) { return; }
+  int first = v.at(0);
+  for (int i = 1; i < static_cast<int>(v.size()); ++i) {
+    v.at(i-1) = v.at(i);
   }
-  *(arr+n-1) = first;
+  v.at(v.size()-1) = first;
 }
 
 #elif defined SLIDE_RIGHT_BUG_2
 // Bug Description: The implementation doesn't copy the last element to the beginning.
 
 void slideRight(std::vector<int> &v) {
-  int *arr = v.data();
-  int n = static_cast<int>(v.size());
-  for (int* p = arr+n-1; arr < p; --p)
-    *p = *(p-1);
+  for (int i = static_cast<int>(v.size()) - 1; i > 0; --i) {
+    v.at(i) = v.at(i-1);
+  }
 }
 
 #elif defined SLIDE_RIGHT_BUG_3
 // Bug Description: The implementation fails for an empty vector
 
 void slideRight(std::vector<int> &v) {
-  if (v.empty()) { v[0] = v[100000]; return; }
-  int *arr = v.data();
-  int n = static_cast<int>(v.size());
-  int last = *(arr+n-1);
-  for (int* p = arr+n-1; arr < p; --p)
-    *p = *(p-1);
-  *arr = last;
+  int last = v.at(v.size() - 1);
+  for (int i = static_cast<int>(v.size()) - 1; i > 0; --i) {
+    v.at(i) = v.at(i-1);
+  }
+  v.at(0) = last;
 }
 
 #else
 
 void slideRight(std::vector<int> &v) {
-  int *arr = v.data();
-  int n = static_cast<int>(v.size());
-  int last = *(arr+n-1);
-  for (int* p = arr+n-1; arr < p; --p)
-    *p = *(p-1);
-  *arr = last;
+  if (v.empty()) { return; }
+  int last = v.at(v.size() - 1);
+  for (int i = static_cast<int>(v.size()) - 1; i > 0; --i) {
+    v.at(i) = v.at(i-1);
+  }
+  v.at(0) = last;
 }
 
 #endif
@@ -89,59 +85,55 @@ void slideRight(std::vector<int> &v) {
 // Bug Description: The function does nothing.
 
 void flip(std::vector<int> &v) {
-  int *arr = v.data();
-  int n = static_cast<int>(v.size());
-  for (int *p = arr+n-1; arr < p; ++arr, --p) {
-    // Accidentally swapping pointers instead of values. arr and p are
+  for (int front = 0, back = static_cast<int>(v.size() - 1); front < back;
+       ++front, --back) {
+    // Accidentally swapping indices instead of values. front and back are
     // swapped and immediately "cross over" each other.
-    int *t = arr;
-    arr = p;
-    p = t;
+    int tmp = front;
+    front = back;
+    back = tmp;
   }
 }
 
 #elif defined FLIP_BUG_2
 // Bug Description: Logical error in swapping middle elements.
-// Only manifests for odd length arrays.
+// Only manifests for odd length vectors.
 
 void flip(std::vector<int> &v) {
-  int *arr = v.data();
   int n = static_cast<int>(v.size());
   for (int i = 0; i < (n-1)/2; ++i) {
-    int t = arr[i];
-    arr[i] = arr[n-1-i];
-    arr[n-1-i] = t;
+    int tmp = v.at(i);
+    v.at(i) = v.at(n-1-i);
+    v.at(n-1-i) = tmp;
   }
 
-  // manually flip middle elements (incorrectly, for odd length arrays)
-  int t = arr[n/2];
-  arr[n/2] = arr[n/2-1];
-  arr[n/2-1] = t;
+  // manually flip middle elements (incorrectly, for odd length vectors)
+  int tmp = v.at(n/2);
+  v.at(n/2) = v.at(n/2-1);
+  v.at(n/2-1) = tmp;
 }
 
 #elif defined FLIP_BUG_3
-// Bug Description: Allows left and right pointers to cross over each other.
-// Pointers will go outside the arrays and eventually crash. Only manifests
-// for even length arrays.
+// Bug Description: Allows left and right indices to cross over each other.
+// Indices will go outside the vectors and eventually crash. Only manifests
+// for even length vectors.
 void flip(std::vector<int> &v) {
-  int *arr = v.data();
-  int n = static_cast<int>(v.size());
-  for (int *p = arr+n-1; arr != p; ++arr, --p) {
-    int t = *arr;
-    *arr = *p;
-    *p = t;
+  for (int front = 0, back = static_cast<int>(v.size()-1); front != back;
+       ++front, --back) {
+    int tmp = v.at(front);
+    v.at(front) = v.at(back);
+    v.at(back) = tmp;
   }
 }
 
 #else
 
 void flip(std::vector<int> &v) {
-  int *arr = v.data();
-  int n = static_cast<int>(v.size());
-  for (int *p = arr+n-1; arr < p; ++arr, --p) {
-    int t = *arr;
-    *arr = *p;
-    *p = t;
+  for (int front = 0, back = static_cast<int>(v.size()-1); front < back;
+       ++front, --back) {
+    int tmp = v.at(front);
+    v.at(front) = v.at(back);
+    v.at(back) = tmp;
   }
 }