Enhance error message for invalid measure.vars in melt

inst/tests/tests.Rraw

@@ -21075,3 +21075,6 @@ setindex(DT, b)
 # make sure that print(DT) doesn't warn due to the header missing index column types, #6806
 # can't use output= here because the print() call is outside withCallingHandlers(...)
 test(2307, { capture.output(print(DT, class = TRUE, show.indices = TRUE)); TRUE })
+
+# test for enhancing error message of invalid column #6512
+test(2308, {msg <- tryCatch({ .Call("uniq_diff", as.integer(c(1, 2, -1, 4)), 4, FALSE); NULL }, error = function(e) e$message)print(msg)return(grepl("\\[-1\\]", msg))}, TRUE)

src/fmelt.c

@@ -176,33 +176,57 @@ bool is_default_measure(SEXP vec) {
 
 // maybe unlist, then unique, then set_diff.
 SEXP uniq_diff(SEXP int_or_list, int ncol, bool is_measure) {
+  // Protect input list/vector, unlisting if necessary
   SEXP int_vec = PROTECT(isNewList(int_or_list) ? unlist_(int_or_list) : int_or_list);
+
+  // Check for duplicated elements in the input vector
   SEXP is_duplicated = PROTECT(duplicated(int_vec, FALSE)); 
+
   int n_unique_cols = 0;
-  for (int i=0; i<length(int_vec); ++i) {
+  SEXP invalid_columns = PROTECT(allocVector(INTSXP, length(int_vec)));
+  int* invalid_col_ptr = INTEGER(invalid_columns);
+  int invalid_count = 0;
+  for (int i = 0; i < length(int_vec); ++i) {
     int col_number = INTEGER(int_vec)[i];
-    bool good_number = 0 < col_number && col_number <= ncol;
-    if (is_measure) good_number |= (col_number==NA_INTEGER);
+    bool good_number = (col_number > 0 && col_number <= ncol);
+    if (is_measure) good_number |= (col_number == NA_INTEGER);
     if (!good_number) {
-      if (is_measure) {
-        error(_("One or more values in 'measure.vars' is invalid."));
-      } else {
-        error(_("One or more values in 'id.vars' is invalid."));
-      }
-    } else if (!LOGICAL(is_duplicated)[i]) n_unique_cols++;
+      invalid_col_ptr[invalid_count++] = col_number;
+    } else if (!LOGICAL(is_duplicated)[i]) {
+      n_unique_cols++;
+    }
+  }
+  if (invalid_count > 0) {
+    char buffer[4096] = "", *nexti = buffer;
+    size_t remaining = sizeof buffer;
+    for (int i = 0; i < invalid_count; ++i) {
+      int offset = snprintf(nexti, remaining, "%s[%d]", i > 0 ? ", " : "", invalid_col_ptr[i]);
+      if (offset < 0 || (size_t)offset >= remaining) break;
+      nexti += offset;
+      remaining -= offset;
+    }
+    error(_("One or more values in '%s' are invalid; please fix by removing: %s"), 
+    is_measure ? "measure.vars" : "id.vars", buffer);
   }
   SEXP unique_col_numbers = PROTECT(allocVector(INTSXP, n_unique_cols)); 
   int unique_i = 0;
-  for (int i=0; i<length(is_duplicated); ++i) {
+
+  // Populate the unique column numbers into the new vector
+  for (int i = 0; i < length(is_duplicated); ++i) {
     if (!LOGICAL(is_duplicated)[i]) {
       INTEGER(unique_col_numbers)[unique_i++] = INTEGER(int_vec)[i];
     }
   }
+
+  // Apply set difference to get final unique column indices
   SEXP out = set_diff(unique_col_numbers, ncol);
-  UNPROTECT(3);
+  UNPROTECT(4);
   return out;
 }
 
+
+
+
 SEXP cols_to_int_or_list(SEXP cols, SEXP dtnames, bool is_measure) {
   switch(TYPEOF(cols)) {
   case STRSXP  : return chmatch(cols, dtnames, 0);