From 868b3d5818379448e2b0273865a4f6aab07c5978 Mon Sep 17 00:00:00 2001
From: Oscar Dowson <odow@users.noreply.github.com>
Date: Thu, 21 Dec 2023 11:26:18 +1300
Subject: [PATCH] Fix modifying user-expressions in macros (#3639)

---
 src/macros.jl       | 13 ++++++++++++-
 test/test_macros.jl | 11 +++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/macros.jl b/src/macros.jl
index d597c311fea..410daeae923 100644
--- a/src/macros.jl
+++ b/src/macros.jl
@@ -231,7 +231,7 @@ A helper function so that we can change how we rewrite expressions in a single
 place and have it cascade to all locations in the JuMP macros that rewrite
 expressions.
 """
-function _rewrite_expression(expr)
+function _rewrite_expression(expr::Expr)
     new_expr = MacroTools.postwalk(_rewrite_to_jump_logic, expr)
     new_aff, parse_aff = _MA.rewrite(new_expr; move_factors_into_sums = false)
     ret = gensym()
@@ -242,6 +242,17 @@ function _rewrite_expression(expr)
     return ret, code
 end
 
+"""
+    _rewrite_expression(expr)
+
+If `expr` is not an `Expr`, then rewriting it won't do anything. We just need to
+copy if it is mutable so that future operations do not modify the user's data.
+"""
+function _rewrite_expression(expr)
+    ret = gensym()
+    return ret, :($ret = $_MA.copy_if_mutable($(esc(expr))))
+end
+
 """
     model_convert(
         model::AbstractModel,
diff --git a/test/test_macros.jl b/test/test_macros.jl
index b0ed162f78b..fbb2baf69d5 100644
--- a/test/test_macros.jl
+++ b/test/test_macros.jl
@@ -2223,4 +2223,15 @@ function test_constraint_broadcast_in_set()
     return
 end
 
+function test_macro_modify_user_data()
+    model = Model()
+    @variable(model, x)
+    @expression(model, e, x + 5)
+    @constraint(model, -10 <= e <= 10)
+    @test isequal_canonical(e, x + 5)
+    @constraint(model, e in MOI.LessThan(1.0))
+    @test isequal_canonical(e, x + 5)
+    return
+end
+
 end  # module