more responses to Vineeth's reviews

third_party/move/move-compiler-v2/src/ast_simplifier.rs

@@ -21,10 +21,13 @@
 //!     - flag uses of uninitialized and unassigned variables
 //! - Implement ExpRewriterFunctions to allow bottom-up replacement
 //!   of some complex expressions by "simpler" ones:
-//!   - Constant folding of operations with constant parameters
-//!   - Eliminate unused variables (with a warning)
+//!   - Constant folding of operations with constant parameters which
+//!     do not abort (have arguments in range).
+//!   - TODO(#12472) Flag In the future, we could warn about expressions
+//!     which are guaranteed to abort, but it's nontrivial so deferred.
+//!   - Eliminate unused expressions (with a warning)
 //!   - Eliminate used variables whose uses are all eliminated by
-//!     constant folding
+//!     other simplification optimizations (e.g., constant folding)
 //!   - Eliminate unused value expressions which are side-effect-free.
 //!   - Unwrap trivial compound expressions:
 //!     - a Sequence of 1 expression
@@ -65,7 +68,7 @@ use std::{
 };
 
 /// Run the AST simplification pass on all target functions in the `env`.
-/// Optionally do some aggressive simplfications that may eliminate code.
+/// Optionally do some aggressive simplifications that may eliminate code.
 pub fn run_simplifier(env: &mut GlobalEnv, eliminate_code: bool) {
     let mut new_definitions = Vec::new(); // Avoid borrowing issues for env.
     for module in env.get_modules() {
@@ -501,7 +504,7 @@ struct SimplifierRewriter<'env> {
 
 // Representation to record a known value of a variable to
 // allow simplification.  Currently we only track constant values
-// and definitely uninitialzed values (from `let` with no binding).
+// and definitely uninitialized values (from `let` with no binding).
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 enum SimpleValue {
     Value(Value),
@@ -912,7 +915,7 @@ impl<'env> ExpRewriterFunctions for SimplifierRewriter<'env> {
         // Simplify binding:
         //   A few ideas for simplification which are implemented below:
         //     (1) if no binding, then simplify to just the body.
-        //     (2) if all pattern vars are unused in body and binding is side-effect free, again return body.
+        //     (2) if all pattern vars are unused in body and binding is OK to remove, again return body.
         //     (3) if some pattern vars are unused in the body, turn them into wildcards.
 
         let pat_vars = pat.vars();
@@ -926,7 +929,8 @@ impl<'env> ExpRewriterFunctions for SimplifierRewriter<'env> {
         }
         let bound_vars = pat.vars();
 
-        // (2) If all pattern vars are unused in body and binding is side-effect free, again return
+        // (2) If all pattern vars are unused in body and binding is OK to remove
+        // (is side-effect free and has no potential impact on Move semantics) , again return
         // body.  But to avoid introducing a drop of a struct value that might not have "drop",
         // also check that opt_binding type has drop.
         let free_vars = body.free_vars();

third_party/move/move-compiler-v2/src/lib.rs

@@ -208,7 +208,6 @@ pub fn run_file_format_gen(env: &GlobalEnv, targets: &FunctionTargetsHolder) ->
 /// Returns the standard env_processor_pipeline
 pub fn create_env_processor_pipeline<'a, 'b>(env: &'a GlobalEnv) -> EnvProcessorPipeline<'b> {
     let options = env.get_extension::<Options>().expect("options");
-    let _safety_on = !options.experiment_on(Experiment::NO_SAFETY);
     let optimize_on = options.experiment_on(Experiment::OPTIMIZE);
 
     let mut env_pipeline = EnvProcessorPipeline::default();
@@ -229,22 +228,14 @@ pub fn create_env_processor_pipeline<'a, 'b>(env: &'a GlobalEnv) -> EnvProcessor
         "access and use check after inlining",
         |env: &mut GlobalEnv| function_checker::check_access_and_use(env, false),
     );
-    env_pipeline.add(
-        "simplifier",
-        if optimize_on {
-            |env: &mut GlobalEnv| {
-                ast_simplifier::run_simplifier(
-                    env, true, // eliminate code only if optimize is on
-                )
-            }
-        } else {
-            |env: &mut GlobalEnv| {
-                ast_simplifier::run_simplifier(
-                    env, false, // eliminate code only if optimize is on
-                )
-            }
-        },
-    );
+    env_pipeline.add("simplifier", {
+        move |env: &mut GlobalEnv| {
+            ast_simplifier::run_simplifier(
+                env,
+                optimize_on, // eliminate code only if optimize is on
+            )
+        }
+    });
     env_pipeline
 }
 

third_party/move/move-compiler-v2/tests/folding/constants_simple.exp

@@ -8,7 +8,7 @@ error: Not a valid constant expression.
   │                     │
   │                     Invalid call or operation in constant
 
-error: Invalid expression in `const`. Operator sub (-) result value out of range for u64
+error: Invalid expression in `const`. Operator `sub (-)` result value out of range for `u64`
   ┌─ tests/folding/constants_simple.move:6:21
   │
 6 │     const C2: u64 = 0 + 1 * 2 % 3 / 4 - 5 >> 6 << 7;