feat(slang): encode require string literal message (#346)

Extract the string literal from the second argument of `require(cond,
"msg")` and pass it into `sol.require`'s `msg` attribute. Previously the
message was always hardcoded to an empty string, causing require reverts
to lose the error reason.

Newly passing tests:

- `tests/solidity/simple/conditional/require.sol` (all)

Author: hedgar2017

Cargo.lock

@@ -69,4 +69,11 @@ MlirType solxCreateContractType(MlirContext ctx, const char *name_ptr,
     return wrap(mlir::sol::ContractType::get(context, name, payable));
 }
 
+MlirType solxCreateStringType(MlirContext ctx, uint32_t dataLocation) {
+    if (dataLocation > 5) abort();
+    auto *context = unwrap(ctx);
+    auto location = static_cast<mlir::sol::DataLocation>(dataLocation);
+    return wrap(mlir::sol::StringType::get(context, location));
+}
+
 } /* extern "C" */

solx-llvm

@@ -53,6 +53,7 @@ use crate::ods::sol::ReturnOperation;
 use crate::ods::sol::RevertOperation;
 use crate::ods::sol::StateVarOperation;
 use crate::ods::sol::StoreOperation;
+use crate::ods::sol::StringLitOperation;
 use crate::ods::sol::WhileOperation;
 use crate::ods::sol::YieldOperation;
 
@@ -311,6 +312,31 @@ impl<'context> Builder<'context> {
             .into()
     }
 
+    // ==== String literals ====
+
+    /// Emits a `sol.string_lit` constant with a `!sol.string<Memory>` result.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the MLIR operation cannot be constructed, indicating a bug in the builder.
+    pub fn emit_sol_string_lit<'block, B>(&self, value: &str, block: &B) -> Value<'context, 'block>
+    where
+        B: BlockLike<'context, 'block>,
+        'context: 'block,
+    {
+        block
+            .append_operation(
+                StringLitOperation::builder(self.context, self.unknown_location)
+                    .value(StringAttribute::new(self.context, value))
+                    .addr(self.types.sol_string_memory)
+                    .build()
+                    .into(),
+            )
+            .result(0)
+            .expect("sol.string_lit always produces one result")
+            .into()
+    }
+
     // ==== Terminators ====
 
     /// Emits a `sol.revert` with an empty signature (no error data).
@@ -352,27 +378,31 @@ impl<'context> Builder<'context> {
         );
     }
 
-    /// Emits a `sol.require` conditional revert with an empty signature.
+    /// Emits a `sol.require` conditional revert with an optional message.
     ///
-    /// Reverts if `condition` is false. Not a terminator — execution continues
-    /// after this op when the condition is true.
+    /// Reverts if `condition` is false. When `msg` is `Some`, the revert
+    /// includes the string as a revert reason. Not a terminator — execution
+    /// continues after this op when the condition is true.
     ///
     /// # Panics
     ///
     /// Panics if the MLIR operation cannot be constructed, indicating a bug in the builder.
-    pub fn emit_sol_require<'block, B>(&self, condition: Value<'context, 'block>, block: &B)
-    where
+    pub fn emit_sol_require<'block, B>(
+        &self,
+        condition: Value<'context, 'block>,
+        msg: Option<&str>,
+        block: &B,
+    ) where
         B: BlockLike<'context, 'block>,
         'context: 'block,
     {
-        block.append_operation(
-            RequireOperation::builder(self.context, self.unknown_location)
-                .cond(condition)
-                .msg(StringAttribute::new(self.context, ""))
-                .args(&[])
-                .build()
-                .into(),
-        );
+        let mut builder = RequireOperation::builder(self.context, self.unknown_location)
+            .cond(condition)
+            .args(&[]);
+        if let Some(msg) = msg {
+            builder = builder.msg(StringAttribute::new(self.context, msg));
+        }
+        block.append_operation(builder.build().into());
     }
 
     /// Emits a `sol.return` terminator.

solx-mlir/sol_attr_stubs.cpp

@@ -26,6 +26,8 @@ pub struct TypeFactory<'context> {
     pub sol_address: Type<'context>,
     /// Sol storage pointer type (`!sol.ptr<ui256, Storage>`).
     pub sol_ptr_storage: Type<'context>,
+    /// Sol memory string type (`!sol.string<Memory>`).
+    pub sol_string_memory: Type<'context>,
 }
 
 impl<'context> TypeFactory<'context> {
@@ -59,13 +61,22 @@ impl<'context> TypeFactory<'context> {
                 solx_utils::DataLocation::Storage as u32,
             ))
         };
+        // SAFETY: `solxCreateStringType` returns a valid MlirType from
+        // the C++ Sol dialect. The context pointer is valid.
+        let sol_string_memory = unsafe {
+            Type::from_raw(crate::ffi::solxCreateStringType(
+                context.to_raw(),
+                solx_utils::DataLocation::Memory as u32,
+            ))
+        };
         Self {
             context,
             i1,
             ui160,
             ui256,
             sol_address,
             sol_ptr_storage,
+            sol_string_memory,
         }
     }
 

solx-mlir/src/context/builder/mod.rs

@@ -104,6 +104,12 @@ unsafe extern "C" {
         payable: bool,
     ) -> mlir_sys::MlirType;
 
+    /// Creates a `sol::StringType` with the given data location.
+    ///
+    /// `data_location` maps to `mlir::sol::DataLocation` (0=Storage, 1=CallData,
+    /// 2=Memory, 3=Stack, 4=Immutable, 5=Transient).
+    pub fn solxCreateStringType(context: MlirContext, data_location: u32) -> mlir_sys::MlirType;
+
     // ---- MLIR core (not in mlir-sys) ----
 
     /// Returns the region that owns the given block.

solx-mlir/src/context/builder/type_factory.rs

@@ -16,7 +16,7 @@ semver.workspace = true
 # `__private_testing_utils` is required by `__private_backend_api` internals
 # (binder/types access in node_extensions). Both are unstable Slang APIs.
 # TODO: pin to a release tag instead of branch = "main"
-slang_solidity = { git = "https://github.com/NomicFoundation/slang.git", rev = "a01bc76b742173941cea1aff0b3b68a9006b38b8", features = ["__private_backend_api", "__private_testing_utils"] }
+slang_solidity = { git = "https://github.com/NomicFoundation/slang.git", rev = "6f892dff8e2d6d275cfe9737c3cb364486c41410", features = ["__private_backend_api", "__private_testing_utils"] }
 melior = { git = "https://github.com/NomicFoundation/melior", rev = "62a909e2ff67fcc19f7f42dc4fb70c3e03376bd2", features = ["ods-dialects", "helpers"] }
 
 solx-codegen-evm = { path = "../solx-codegen-evm" }

solx-mlir/src/ffi.rs

@@ -132,7 +132,7 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
                     .state
                     .builder
                     .emit_sol_cmp(value, zero, CmpPredicate::Eq, &block);
-                let result_type = target_type.unwrap_or_else(|| self.state.builder.types.ui256);
+                let result_type = target_type.unwrap_or(self.state.builder.types.ui256);
                 let result = TypeConversion::from_target_type(result_type, &self.state.builder)
                     .emit(cmp, &self.state.builder, &block);
                 Ok((result, block))

solx-slang/Cargo.toml

@@ -102,12 +102,18 @@ impl<'emitter, 'state, 'context, 'block> CallEmitter<'emitter, 'state, 'context,
             && (positional_arguments.len() == 1
                 || positional_arguments.len() == Self::MAX_REQUIRE_ARGUMENTS)
         {
-            // TODO: encode revert reason from second argument
-            let first = positional_arguments
-                .iter()
-                .next()
-                .expect("length checked above");
-            let block = self.emit_require(&first, block)?;
+            let mut args = positional_arguments.iter();
+            let condition = args.next().expect("length checked above");
+            let message_arg = args.next();
+            let message = match &message_arg {
+                Some(Expression::StringExpression(string_expression)) => {
+                    let bytes = string_expression.value();
+                    Some(String::from_utf8(bytes).expect("require message is valid UTF-8"))
+                }
+                Some(_) => anyhow::bail!("require message must be a string literal"),
+                None => None,
+            };
+            let block = self.emit_require(&condition, message.as_deref(), block)?;
             return Ok((None, block));
         }
 
@@ -242,10 +248,12 @@ impl<'emitter, 'state, 'context, 'block> CallEmitter<'emitter, 'state, 'context,
         Ok(block)
     }
 
-    /// Emits a `require(condition)` built-in via `sol.require`.
+    /// Emits a `require(condition)` or `require(condition, "message")` built-in
+    /// via `sol.require`.
     fn emit_require(
         &self,
         condition: &Expression,
+        message: Option<&str>,
         block: BlockRef<'context, 'block>,
     ) -> anyhow::Result<BlockRef<'context, 'block>> {
         let (condition_value, block) = self.expression_emitter.emit_value(condition, block)?;
@@ -256,7 +264,7 @@ impl<'emitter, 'state, 'context, 'block> CallEmitter<'emitter, 'state, 'context,
         self.expression_emitter
             .state
             .builder
-            .emit_sol_require(condition_boolean, &block);
+            .emit_sol_require(condition_boolean, message, &block);
 
         Ok(block)
     }

solx-slang/src/ast/contract/function/expression/arithmetic.rs

@@ -154,6 +154,12 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
                     .into();
                 Ok((Some(value), block))
             }
+            Expression::StringExpression(string_expression) => {
+                let bytes = string_expression.value();
+                let text = std::str::from_utf8(&bytes).expect("string literal is valid UTF-8");
+                let value = self.state.builder.emit_sol_string_lit(text, &block);
+                Ok((Some(value), block))
+            }
             Expression::Identifier(identifier) => {
                 let name = identifier.name();
                 match identifier.resolve_to_definition() {
@@ -302,7 +308,7 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
             Expression::ConditionalExpression(conditional) => {
                 let result_type = self
                     .resolve_expression_type(conditional.node_id())
-                    .unwrap_or_else(|| self.state.builder.types.ui256);
+                    .unwrap_or(self.state.builder.types.ui256);
                 let condition = conditional.operand();
                 let (condition_value, block) = self.emit_value(&condition, block)?;
                 let condition_boolean = self.emit_is_nonzero(condition_value, &block);