Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Result Type Support #687

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl Ast {
/// Returns a reference to the AST [node](Node) with the provided identifier, if one exists.
/// The identifier must be fully qualified, since this performs no scope resolution, but cannot begin with '::'.
///
/// Anonymous types (those without identifiers) cannot be looked up. This only includes sequences and dictionaries.
/// Anonymous types (those without identifiers) cannot be looked up. These are results, sequences, and dictionaries.
/// Primitive types can be looked up by their Slice keywords. Care should be taken when looking up modules (which
/// can be re-opened) or parameters and return members (which share an AST scope), since these may not be unique.
///
Expand Down Expand Up @@ -148,7 +148,7 @@ impl Ast {
/// This returns the first matching AST node it can find. If another node in a more outward scope also has the
/// specified identifier, it is shadowed, and will not be returned.
///
/// Anonymous types (those without identifiers) cannot be looked up. This only includes sequences and dictionaries.
/// Anonymous types (those without identifiers) cannot be looked up. These are results, sequences, and dictionaries.
/// Primitive types can be looked up by their Slice keywords. Care should be taken when looking up modules (which
/// can be re-opened) or parameters and return members (which share an AST scope), since these may not be unique.
///
Expand Down Expand Up @@ -201,7 +201,7 @@ impl Ast {
/// Returns a reference to a Slice element with the provided identifier and specified type, if one exists.
/// The identifier must be fully qualified, since this performs no scope resolution, but cannot begin with '::'.
///
/// Anonymous types (those without identifiers) cannot be looked up. This only includes sequences and dictionaries.
/// Anonymous types (those without identifiers) cannot be looked up. These are results, sequences, and dictionaries.
/// Primitive types can be looked up by their Slice keywords. Care should be taken when looking up modules (which
/// can be re-opened) or parameters and return members (which share an AST scope), since these may not be unique.
///
Expand Down Expand Up @@ -255,7 +255,7 @@ impl Ast {
/// This returns the first matching Slice element it can find. If another element in a more outward scope also has
/// the specified identifier, it is shadowed, and will not be returned.
///
/// Anonymous types (those without identifiers) cannot be looked up. This only includes sequences and dictionaries.
/// Anonymous types (those without identifiers) cannot be looked up. These are results, sequences, and dictionaries.
/// Primitive types can be looked up by their Slice keywords. Care should be taken when looking up modules (which
/// can be re-opened) or parameters and return members (which share an AST scope), since these may not be unique.
///
Expand Down
5 changes: 4 additions & 1 deletion src/ast/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ macro_rules! generate_node_enum {
// generate the `Node` enum with variants for every type allowed to be in the AST.
generate_node_enum! {
Module, Struct, Class, Exception, Field, Interface, Operation, Parameter, Enum,
Enumerator, CustomType, TypeAlias, Sequence, Dictionary, Primitive, Attribute
Enumerator, CustomType, TypeAlias, ResultType, Sequence, Dictionary, Primitive, Attribute
}

impl<'a> TryFrom<&'a Node> for WeakPtr<dyn Type> {
Expand All @@ -100,6 +100,7 @@ impl<'a> TryFrom<&'a Node> for WeakPtr<dyn Type> {
Node::Enum(enum_ptr) => Ok(downgrade_as!(enum_ptr, dyn Type)),
Node::CustomType(custom_type_ptr) => Ok(downgrade_as!(custom_type_ptr, dyn Type)),
Node::TypeAlias(type_alias_ptr) => Ok(downgrade_as!(type_alias_ptr, dyn Type)),
Node::ResultType(result_ptr) => Ok(downgrade_as!(result_ptr, dyn Type)),
Node::Sequence(sequence_ptr) => Ok(downgrade_as!(sequence_ptr, dyn Type)),
Node::Dictionary(dictionary_ptr) => Ok(downgrade_as!(dictionary_ptr, dyn Type)),
Node::Primitive(primitive_ptr) => Ok(downgrade_as!(primitive_ptr, dyn Type)),
Expand All @@ -126,6 +127,7 @@ impl<'a> TryFrom<&'a Node> for &'a dyn Type {
Node::Enum(enum_ptr) => Ok(enum_ptr.borrow()),
Node::CustomType(custom_type_ptr) => Ok(custom_type_ptr.borrow()),
Node::TypeAlias(type_alias_ptr) => Ok(type_alias_ptr.borrow()),
Node::ResultType(result_ptr) => Ok(result_ptr.borrow()),
Node::Sequence(sequence_ptr) => Ok(sequence_ptr.borrow()),
Node::Dictionary(dictionary_ptr) => Ok(dictionary_ptr.borrow()),
Node::Primitive(primitive_ptr) => Ok(primitive_ptr.borrow()),
Expand Down Expand Up @@ -253,6 +255,7 @@ impl_into_node_for!(Enum);
impl_into_node_for!(Enumerator);
impl_into_node_for!(CustomType);
impl_into_node_for!(TypeAlias);
impl_into_node_for!(ResultType);
impl_into_node_for!(Sequence);
impl_into_node_for!(Dictionary);
// We don't implement it on `Primitive`, because primitive types are baked into the compiler, so we don't need
Expand Down
2 changes: 1 addition & 1 deletion src/diagnostics/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub enum Lint {
IncorrectDocComment { message: String },

/// A link in a doc-comment couldn't be resolved. Either:
/// - The link pointed to an un-linkable element, e.g. a module, primitive, sequence, or dictionary.
/// - The link pointed to an un-linkable element, e.g. a module, result, sequence, dictionary, or primitive.
/// - The link pointed to a non-existent element.
BrokenDocLink { message: String },
}
Expand Down
2 changes: 2 additions & 0 deletions src/grammar/elements/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod module;
mod operation;
mod parameter;
mod primitive;
mod result;
mod sequence;
mod r#struct;
mod type_alias;
Expand All @@ -39,6 +40,7 @@ pub use self::parameter::*;
pub use self::primitive::*;
pub use self::r#enum::*;
pub use self::r#struct::*;
pub use self::result::*;
pub use self::sequence::*;
pub use self::type_alias::*;
pub use self::type_ref::*;
41 changes: 41 additions & 0 deletions src/grammar/elements/result.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) ZeroC, Inc.

use super::super::*;
use crate::supported_encodings::SupportedEncodings;

#[derive(Debug)]
pub struct ResultType {
pub ok_type: TypeRef,
pub err_type: TypeRef,
}

impl Type for ResultType {
fn type_string(&self) -> String {
format!(
"Result<{}, {}>",
self.ok_type.type_string(),
self.err_type.type_string(),
)
}

fn fixed_wire_size(&self) -> Option<u32> {
None
}

fn is_class_type(&self) -> bool {
false
}

fn tag_format(&self) -> Option<TagFormat> {
unreachable!("tag format was called on a Slice2 only type!")
}

fn supported_encodings(&self) -> SupportedEncodings {
let mut encodings = self.ok_type.supported_encodings();
encodings.intersect_with(&self.err_type.supported_encodings());
encodings.disable(Encoding::Slice1);
encodings
}
}

implement_Element_for!(ResultType, "result");
2 changes: 1 addition & 1 deletion src/grammar/wrappers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,4 @@ pub trait AsTypes {
fn concrete_type(&self) -> Types;
}

generate_types_wrapper!(Struct, Class, Enum, CustomType, Sequence, Dictionary, Primitive);
generate_types_wrapper!(Struct, Class, Enum, CustomType, ResultType, Sequence, Dictionary, Primitive);
8 changes: 8 additions & 0 deletions src/parsers/slice/grammar.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ extern {
enum_keyword => TokenKind::EnumKeyword,
custom_keyword => TokenKind::CustomKeyword,
type_alias_keyword => TokenKind::TypeAliasKeyword,
result_keyword => TokenKind::ResultKeyword,

// Collection keywords
sequence_keyword => TokenKind::SequenceKeyword,
Expand Down Expand Up @@ -218,6 +219,12 @@ TypeAlias: OwnedPtr<TypeAlias> = {
},
}

Result: OwnedPtr<ResultType> = {
result_keyword "<" <ok_type: TypeRef> "," <err_type: TypeRef> ">" => {
OwnedPtr::new(ResultType { ok_type, err_type })
},
}

Sequence: OwnedPtr<Sequence> = {
sequence_keyword "<" <element_type: TypeRef> ">" => {
OwnedPtr::new(Sequence { element_type })
Expand Down Expand Up @@ -258,6 +265,7 @@ TypeRef: TypeRef = {

TypeRefDefinition: TypeRefDefinition = {
Primitive => primitive_to_type_ref_definition(parser, <>),
Result => anonymous_type_to_type_ref_definition(parser, <>),
Sequence => anonymous_type_to_type_ref_definition(parser, <>),
Dictionary => anonymous_type_to_type_ref_definition(parser, <>),
RelativeIdentifier => construct_unpatched_type_ref_definition(<>),
Expand Down
1 change: 1 addition & 0 deletions src/parsers/slice/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ where
"enum" => TokenKind::EnumKeyword,
"custom" => TokenKind::CustomKeyword,
"typealias" => TokenKind::TypeAliasKeyword,
"Result" => TokenKind::ResultKeyword,
"Sequence" => TokenKind::SequenceKeyword,
"Dictionary" => TokenKind::DictionaryKeyword,
"bool" => TokenKind::BoolKeyword,
Expand Down
1 change: 1 addition & 0 deletions src/parsers/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ fn generate_message(expected: &[String], found: impl std::fmt::Display) -> Strin
"enum_keyword" => tokens::TokenKind::EnumKeyword.to_string(),
"custom_keyword" => tokens::TokenKind::CustomKeyword.to_string(),
"type_alias_keyword" => tokens::TokenKind::TypeAliasKeyword.to_string(),
"result_keyword" => tokens::TokenKind::ResultKeyword.to_string(),

// Collection keywords
"sequence_keyword" => tokens::TokenKind::SequenceKeyword.to_string(),
Expand Down
2 changes: 2 additions & 0 deletions src/parsers/slice/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub enum TokenKind<'input> {
EnumKeyword, // "enum"
CustomKeyword, // "custom"
TypeAliasKeyword, // "typealias"
ResultKeyword, // "Result"

// Collection keywords
SequenceKeyword, // "Sequence"
Expand Down Expand Up @@ -110,6 +111,7 @@ impl fmt::Display for TokenKind<'_> {
Self::EnumKeyword => "enum",
Self::CustomKeyword => "custom",
Self::TypeAliasKeyword => "typealias",
Self::ResultKeyword => "Result",
Self::SequenceKeyword => "Sequence",
Self::DictionaryKeyword => "Dictionary",
Self::BoolKeyword => "bool",
Expand Down
38 changes: 38 additions & 0 deletions src/patchers/encoding_patcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ impl EncodingPatcher<'_> {
allow_nullable_with_slice_1 = true;
self.get_supported_encodings_for(custom_type)
}
Types::ResultType(result_type) => {
let mut encodings = SupportedEncodings::dummy();
let additional_info = result_type.compute_supported_encodings(self, &mut encodings, compilation_mode);
if let Some(note) = additional_info {
let diagnostic = Diagnostic::new(Error::UnsupportedType {
kind: type_ref.type_string(),
mode: compilation_mode,
})
.set_span(type_ref.span())
.add_note(note, None)
.extend_notes(self.get_mode_mismatch_note(type_ref));
diagnostics.push(diagnostic);
}
encodings
}
Types::Sequence(sequence) => {
// Sequences are supported by any encoding that supports their elements.
self.get_supported_encodings_for_type_ref(&sequence.element_type, compilation_mode, false, None)
Expand Down Expand Up @@ -459,3 +474,26 @@ impl ComputeSupportedEncodings for TypeAlias {
None
}
}

impl ComputeSupportedEncodings for ResultType {
fn compute_supported_encodings(
&self,
patcher: &mut EncodingPatcher,
supported_encodings: &mut SupportedEncodings,
compilation_mode: CompilationMode,
) -> Option<&'static str> {
// Results only support encodings that their `ok` and `err` types also support.
let ok_encodings = patcher.get_supported_encodings_for_type_ref(&self.ok_type, compilation_mode, false, None);
supported_encodings.intersect_with(&ok_encodings);
let err_encodings = patcher.get_supported_encodings_for_type_ref(&self.err_type, compilation_mode, false, None);
supported_encodings.intersect_with(&err_encodings);

// Result can only be used in Slice2 mode.
supported_encodings.disable(Encoding::Slice1);
if compilation_mode == CompilationMode::Slice1 {
return Some("'Result' can only be used in Slice2 mode");
}

None
}
}
16 changes: 16 additions & 0 deletions src/patchers/type_ref_patcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ impl TypeRefPatcher<'_> {
self.resolve_definition(type_ref, ast)
.map(PatchKind::TypeAliasUnderlyingType)
}
Node::ResultType(result_ptr) => {
let result_type = result_ptr.borrow();
let ok_patch = self.resolve_definition(&result_type.ok_type, ast);
let err_patch = self.resolve_definition(&result_type.err_type, ast);
Some(PatchKind::ResultTypes(ok_patch, err_patch))
}
Node::Sequence(sequence_ptr) => {
let type_ref = &sequence_ptr.borrow().element_type;
self.resolve_definition(type_ref, ast).map(PatchKind::SequenceType)
Expand Down Expand Up @@ -154,6 +160,15 @@ impl TypeRefPatcher<'_> {
let type_alias_underlying_type_ref = &mut type_alias_ptr.borrow_mut().underlying;
type_alias_underlying_type_ref.patch(type_alias_underlying_type_ptr, attributes);
}
PatchKind::ResultTypes(ok_patch, err_patch) => {
let result_ptr: &mut OwnedPtr<ResultType> = element.try_into().unwrap();
if let Some((ok_type_ptr, ok_attributes)) = ok_patch {
result_ptr.borrow_mut().ok_type.patch(ok_type_ptr, ok_attributes);
}
if let Some((err_type_ptr, err_attributes)) = err_patch {
result_ptr.borrow_mut().err_type.patch(err_type_ptr, err_attributes);
}
}
PatchKind::SequenceType((element_type_ptr, attributes)) => {
let sequence_ptr: &mut OwnedPtr<Sequence> = element.try_into().unwrap();
let element_type_ref = &mut sequence_ptr.borrow_mut().element_type;
Expand Down Expand Up @@ -332,6 +347,7 @@ enum PatchKind {
ExceptionSpecification(Vec<Patch<Exception>>),
EnumUnderlyingType(Patch<Primitive>),
TypeAliasUnderlyingType(Patch<dyn Type>),
ResultTypes(Option<Patch<dyn Type>>, Option<Patch<dyn Type>>),
SequenceType(Patch<dyn Type>),
DictionaryTypes(Option<Patch<dyn Type>>, Option<Patch<dyn Type>>),
}
Expand Down
4 changes: 2 additions & 2 deletions src/utils/code_gen_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ pub enum TypeContext {
/// Used when generating the types of operation parameters, and return types in places where
/// they're being encoded.
Encode,
/// Used when generating types that are parts of other types, such as the key & value types of
/// dictionaries, or the element type of a sequence.
/// Used when generating types that are parts of other types, such as the ok & err types of results,
/// the key & value types of dictionaries, or the element type of a sequence.
Nested,
}

Expand Down
1 change: 1 addition & 0 deletions src/validators/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ fn check_dictionary_key_type(type_ref: &TypeRef) -> Option<Diagnostic> {
Types::Class(_) => false,
Types::Enum(_) => true,
Types::CustomType(_) => true,
Types::ResultType(_) => false,
Types::Sequence(_) => false,
Types::Dictionary(_) => false,
Types::Primitive(primitive) => {
Expand Down
1 change: 1 addition & 0 deletions src/validators/members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ fn tagged_members_cannot_use_classes(members: Vec<&impl Member>, diagnostics: &m
Types::Class(_) => true,
Types::Enum(_) => false,
Types::CustomType(_) => false,
Types::ResultType(_) => false, // 'Result' is Slice2 only, and classes are Slice1 only.
Types::Sequence(sequence) => uses_classes(&sequence.element_type),
// It is disallowed for key types to use classes, so we only need to check the value type.
Types::Dictionary(dictionary) => uses_classes(&dictionary.value_type),
Expand Down
14 changes: 10 additions & 4 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,19 +272,25 @@ impl Enumerator {
impl TypeRef {
/// Visits the [TypeRef] with the provided `visitor`.
///
/// This function first calls `visitor.visit_type_ref`, then if the type being referenced is a sequence or
/// dictionary, it recursively calls itself on their underlying element, key, and value types.
/// This function first calls `visitor.visit_type_ref`, then if the type being referenced is a result, sequence,
/// or dictionary, it recursively calls itself on their underlying types.
pub fn visit_with(&self, visitor: &mut impl Visitor) {
visitor.visit_type_ref(self);

// If this typeref isn't patched, do not attempt to visit it further.
// Note that sequence and dictionary types (the only types we visit further) are always patched anyways.
// Note that result, sequence, and dictionary types (the only ones we visit further) are always patched anyways.
if matches!(&self.definition, TypeRefDefinition::Unpatched(_)) {
return;
}

match self.concrete_type() {
Types::Sequence(sequence_ref) => sequence_ref.element_type.visit_with(visitor),
Types::ResultType(result_ref) => {
result_ref.ok_type.visit_with(visitor);
result_ref.err_type.visit_with(visitor);
}
Types::Sequence(sequence_ref_______________) => {
sequence_ref_______________.element_type.visit_with(visitor)
}
Types::Dictionary(dictionary_ref) => {
dictionary_ref.key_type.visit_with(visitor);
dictionary_ref.value_type.visit_with(visitor);
Expand Down
1 change: 1 addition & 0 deletions tests/attribute_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ mod attributes {

#[test_case("[deprecated] string"; "non nested")]
#[test_case("Sequence<[deprecated] string>"; "nested")]
#[test_case("Result<bool, [deprecated] varuint62>"; "nested result")]
fn attributes_on_anonymous_types_are_rejected(alias_type: &str) {
let slice = format!(
"
Expand Down
7 changes: 2 additions & 5 deletions tests/interfaces/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn can_have_return_value() {
module Test

interface I {
op() -> string
op() -> Result<bool, string>
}
";

Expand All @@ -141,10 +141,7 @@ fn can_have_return_value() {

assert_eq!(returns.len(), 1);
assert_eq!(returns[0].identifier(), "returnValue");
assert!(matches!(
returns[0].data_type.concrete_type(),
Types::Primitive(Primitive::String),
));
assert!(matches!(returns[0].data_type.concrete_type(), Types::ResultType(_)));
}

#[test]
Expand Down
Loading