From caaf51976df0ca14ba3e9b692855a1ecee77a267 Mon Sep 17 00:00:00 2001
From: Kasper Peeters <kasper.peeters@cadabra.science>
Date: Sun, 4 Feb 2024 17:52:32 +0000
Subject: [PATCH] Tighten conditions on when to accept integer values and
 sequence values for Indices.

---
 core/properties/Indices.cc | 26 +++++++++++++++++---------
 tests/dummies.cdb          | 13 ++++++++++++-
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/core/properties/Indices.cc b/core/properties/Indices.cc
index 2e61c2fbd2..bb3468c951 100644
--- a/core/properties/Indices.cc
+++ b/core/properties/Indices.cc
@@ -8,6 +8,8 @@
 
 using namespace cadabra;
 
+#define MAX_INTEGER_RANGE 100
+
 Indices::Indices()
 	: position_type(free)
 	{
@@ -66,24 +68,30 @@ bool Indices::parse(Kernel& kernel, std::shared_ptr<Ex> ex, keyval_t& keyvals)
 		else if(ki->first=="values") {
 			//std::cerr << "got values keyword " << *(ki->second->name) << std::endl;
 			if(*ki->second->name=="\\sequence") {
+				// Only accept a sequence if both start and end are explicit integers.
+				Ex::sibling_iterator sqit1 = Ex::begin(ki->second);
+				Ex::sibling_iterator sqit2 = sqit1;
+				++sqit2;
+				if(!sqit1->is_integer() || !sqit2->is_integer()) 
+					throw ConsistencyException("Value sequence for Indices property not explicit integers.");
+
 				auto args = std::make_shared<cadabra::Ex>(ki->second);
 				auto prop = new Integer();
 				kernel.inject_property(prop, ex, args);
 
-				if (prop->from.is_integer() && prop->to.is_integer()) {
-					if (prop->difference.to_integer() < 100) {
-						for (int i=prop->from.to_integer(); i<=prop->to.to_integer(); ++i) {
-							values.push_back(Ex(i));
-							}
-						}
-					}
+				if(prop->difference.to_integer() > MAX_INTEGER_RANGE)
+					throw ConsistencyException("Value sequence for Indices property spans too many elements.");
 
+				for (int i=prop->from.to_integer(); i<=prop->to.to_integer(); ++i) {
+					values.push_back(Ex(i));
+					}
+				
 				++ki;
 				continue;
 				}
 
 			collect_index_values(ki->second);
-			// If all values are indices, add an `Integer' property for the object,
+			// If all values are integers, add an `Integer' property for the object,
 			// listing these integers.
 			bool is_number=true;
 			bool is_continuous=false;
@@ -98,7 +106,7 @@ bool Indices::parse(Kernel& kernel, std::shared_ptr<Ex> ex, keyval_t& keyvals)
 					});
 				is_continuous = (int)values.size() == (values[values.size() - 1].to_integer() - values[0].to_integer() + 1);
 				}
-			// FIXME: do not apply Integer to a list of integers with gaps as
+			// Do not apply Integer to a list of integers with gaps as
 			// the former can only deal with continuous ranges.
 			if(is_continuous) {
 //				std::cerr << "Injecting Integer property" << std::endl;
diff --git a/tests/dummies.cdb b/tests/dummies.cdb
index 3e27785eff..2eff61d127 100644
--- a/tests/dummies.cdb
+++ b/tests/dummies.cdb
@@ -90,15 +90,26 @@ test06()
 
 def test07():
    __cdbkernel__ = create_scope()
+   try:
+      {r, s}::Indices(values={-1..a}).
+      sys.exit(-1)
+   except:
+      print("Test 07a passed")      
+   try:
+      {r, s}::Indices(values={-1..999}).
+      sys.exit(-1)
+   except:
+      print("Test 07b passed")      
    {a, b}::Indices(values={-1..2}).
    {c, d}::Indices(values={-1,1,2}).
    \delta{#}::KroneckerDelta.
    ex1 := \delta_{a b} \delta_{a b}.
    eliminate_kronecker(ex1)
    assert ex1 == 4
+   print("Test 07c passed")
    ex2 := \delta_{c d} \delta_{c d}.
    eliminate_kronecker(ex2)
    assert ex2 == $\delta_{d d}$
-   print("Test 07 passed")
+   print("Test 07d passed")
 
 test07()