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

Additional annotation parsing #919

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
6 changes: 6 additions & 0 deletions src/idl/include/idl/tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
/* if no explicit default is specified and range is not covered */
#define IDL_IMPLICIT_DEFAULT_CASE_LABEL (IDL_DEFAULT_CASE_LABEL | 2u)
#define IDL_ENUMERATOR (1llu<<26)
#define IDL_DEFAULT_ENUMERATOR (IDL_ENUMERATOR | 1u)
#define IDL_IMPLICIT_DEFAULT_ENUMERATOR (IDL_DEFAULT_ENUMERATOR | 2u)
#define IDL_DECLARATOR (1llu<<25)
/* annotations */
#define IDL_ANNOTATION (1llu<<24)
Expand Down Expand Up @@ -282,6 +284,8 @@ struct idl_member {
idl_declarator_t *declarators;
/* metadata */
IDL_ANNOTATABLE(bool) key;
IDL_ANNOTATABLE(bool) optional;
IDL_ANNOTATABLE(const idl_literal_t*) value;
IDL_ANNOTATABLE(uint32_t) id;
};

Expand Down Expand Up @@ -385,6 +389,7 @@ struct idl_enum {
idl_node_t node;
struct idl_name *name;
idl_enumerator_t *enumerators;
idl_enumerator_t *default_enumerator;
IDL_ANNOTATABLE(idl_extensibility_t) extensibility;
};

Expand Down Expand Up @@ -481,6 +486,7 @@ IDL_EXPORT bool idl_identifier_is(const void *node, const char *identifier);
IDL_EXPORT const idl_name_t *idl_name(const void *node);
IDL_EXPORT uint32_t idl_array_size(const void *node);
IDL_EXPORT uint32_t idl_bound(const void *node);
IDL_EXPORT const idl_literal_t *idl_default_value(const void *node);

/* navigation */
IDL_EXPORT void *idl_ancestor(const void *node, size_t levels);
Expand Down
132 changes: 122 additions & 10 deletions src/idl/src/annotation.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,44 @@ annotate_autoid(
return IDL_RETCODE_OK;
}

static idl_retcode_t
annotate_optional(
idl_pstate_t *pstate,
idl_annotation_appl_t *annotation_appl,
idl_node_t *node)
{
const idl_const_expr_t *const_expr;
idl_member_t *mem = (idl_member_t*)node;
bool value = true;

assert(annotation_appl);

if (!idl_is_member(node)) {
idl_error(pstate, idl_location(annotation_appl),
"@optional can only be assigned to members");
return IDL_RETCODE_SEMANTIC_ERROR;
} else if (mem->key.value) {
idl_error(pstate, idl_location(annotation_appl),
"@optional cannot be assigned to key members");
return IDL_RETCODE_SEMANTIC_ERROR;
} else if (mem->value.annotation) {
idl_error(pstate, idl_location(annotation_appl),
"@optional cannot be assigned to members with explicit default values");
return IDL_RETCODE_SEMANTIC_ERROR;
}

if (annotation_appl->parameters) {
const_expr = annotation_appl->parameters->const_expr;
assert(const_expr);
value = ((const idl_literal_t*)const_expr)->value.bln;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't ((idl_member_t *)node)->is_optional be set at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer relevant, after rebasing I have changed to using the IDL_ANNOTATABLE macro

mem->optional.annotation = annotation_appl;
mem->optional.value = value;

return IDL_RETCODE_OK;
}

static idl_retcode_t
annotate_value(
idl_pstate_t *pstate,
Expand Down Expand Up @@ -250,8 +288,13 @@ annotate_key(
}

if (idl_mask(node) & IDL_MEMBER) {
((idl_member_t *)node)->key.annotation = annotation_appl;
((idl_member_t *)node)->key.value = key;
if (((idl_member_t*)node)->optional.value) {
idl_error(pstate, idl_location(annotation_appl),
"@key cannot be applied to optional members");
return IDL_RETCODE_SEMANTIC_ERROR;
}
((idl_member_t*)node)->key.annotation = annotation_appl;
((idl_member_t*)node)->key.value = key;
} else if (idl_mask(node) & IDL_SWITCH_TYPE_SPEC) {
((idl_switch_type_spec_t *)node)->key.annotation = annotation_appl;
((idl_switch_type_spec_t *)node)->key.value = key;
Expand Down Expand Up @@ -306,6 +349,70 @@ set_nested(
return IDL_RETCODE_OK;
}

static idl_retcode_t
annotate_default(
idl_pstate_t *pstate,
idl_annotation_appl_t *annotation_appl,
idl_node_t *node)
{
idl_const_expr_t *value;
idl_member_t *mem = (idl_member_t*)node;
idl_type_spec_t *mem_spec = mem->type_spec;

assert(annotation_appl);
assert(annotation_appl->parameters);

if (!idl_is_member(node)) {
idl_error(pstate, idl_location(annotation_appl),
"@default can only be assigned to members");
return IDL_RETCODE_SEMANTIC_ERROR;
} else if (mem->optional.value) {
idl_error(pstate, idl_location(annotation_appl),
"@default cannot be set on optional members");
return IDL_RETCODE_SEMANTIC_ERROR;
}

value = annotation_appl->parameters->const_expr;
assert(idl_is_literal(value));

/*check whether type of literal matches and falls inside spec of member*/
idl_type_t mem_type = idl_type(mem_spec);
if (mem_type != idl_type(value)) {
idl_retcode_t ret = IDL_RETCODE_OK;
idl_literal_t *literal = NULL;
if ((ret = idl_evaluate(pstate, value, mem_type, &literal)))
return ret;

assert(literal);
annotation_appl->parameters->const_expr = literal;
literal->node.parent = (idl_node_t*)annotation_appl->parameters;
}

((idl_member_t *)node)->value.annotation = annotation_appl;
((idl_member_t *)node)->value.value = annotation_appl->parameters->const_expr;

return IDL_RETCODE_OK;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to move the @default implementation to a different PR (including the changes below for @default in the annotations array)? Or fix the implementation in this PR (and change the PR title)

static idl_retcode_t
annotate_default_literal(
idl_pstate_t *pstate,
idl_annotation_appl_t *annotation_appl,
idl_node_t *node)
{
assert(pstate);

if (!idl_is_enumerator(node)) {
idl_error(pstate, idl_location(annotation_appl),
"@default_literal can only be assigned to enumerators");
return IDL_RETCODE_SEMANTIC_ERROR;
}

node->mask |= IDL_DEFAULT_ENUMERATOR;

return IDL_RETCODE_OK;
}

static idl_retcode_t
annotate_nested(
idl_pstate_t *pstate,
Expand Down Expand Up @@ -430,12 +537,11 @@ static const idl_builtin_annotation_t annotations[] = {
"instructs to automatically allocate identifiers to elements that have "
"not been assigned a 32-bit unsigned identifiers explicitly.</p>",
.callback = &annotate_autoid },
#if 0
{ .syntax = "@annotation optional { boolean value default TRUE; };",
.summary =
"<p>Set optionality on any element that makes sense to be optional.</p>",
.callback = annotate_optional },
#endif
"<p>Indicates that the annotated member may be in a NULL state, not"
"containing any value.</p>",
.callback = &annotate_optional },
{ .syntax = "@annotation value { any value; };",
.summary =
"<p>Set a constant value to any element that may be given a constant "
Expand Down Expand Up @@ -466,17 +572,23 @@ static const idl_builtin_annotation_t annotations[] = {
"<p>Specify a data member is part of the key for the object whose type "
"is the constructed data type owning this element.</p>",
.callback = annotate_key },
{ .syntax = "@annotation default { any value; };",
.summary =
"<p>Specify the value with which the annotated member should be default"
"initialized.</p>",
.callback = &annotate_default },
{ .syntax = "@annotation default_literal { };",
.summary =
"<p>Explicity sets the default value for an enum to the annotated enumerator"
"instead of the first entry.</p>",
.callback = &annotate_default_literal },
#if 0
{ .syntax = "@annotation must_understand { boolean value default TRUE; };",
.summary =
"<p>Specify the data member must be understood by any application "
"making use of that piece of data.</p>",
.callback = annotate_must_understand },
/* units and ranges */
{ .syntax = "@annotation default { any value; };",
.summary =
"<p>Specify a default value for the annotated element.</p>",
.callback = annotate_default },
{ .syntax = "@annotation range { any min; any max; };",
.summary =
"<p>Specify a range of allowed value for the annotated element.</p>",
Expand Down
2 changes: 1 addition & 1 deletion src/idl/src/expression.c
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ idl_evaluate(
}
}

if ((ret = idl_create_literal(pstate, idl_location(const_expr), type, nodep)))
if ((ret = idl_create_literal(pstate, idl_location(const_expr), implicit, nodep)))
return ret;
(*((idl_literal_t **)nodep))->value = temporary.value;
done:
Expand Down
2 changes: 2 additions & 0 deletions src/idl/src/scanner.c
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,8 @@ tokenize(
/* annotation names cannot be keywords, i.e. "@default" */
if (pstate->scanner.state == IDL_SCAN_ANNOTATION_APPL_NAME)
goto identifier;
if (pstate->scanner.state == IDL_SCAN_ANNOTATION_APPL_SCOPED_NAME)
goto identifier;
if (pstate->scanner.state == IDL_SCAN_ANNOTATION_NAME)
goto identifier;
if ((code = idl_iskeyword(pstate, str, !(pstate->flags & IDL_FLAG_CASE_SENSITIVE))))
Expand Down
35 changes: 34 additions & 1 deletion src/idl/src/tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,19 @@ uint32_t idl_bound(const void *node)
return 0u;
}

const idl_literal_t *idl_default_value(const void *node)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the function name idl_has_default I would expect a bool that indicates if the node has a default (which is what the function idl_is_default below does). Maybe a better name would be idl_get_default_value or something similar?

if (idl_is_member(node)) {
return ((const idl_member_t*)node)->value.value;
} else if (idl_is_declarator(node)) {
const idl_node_t *parent = idl_parent(node);
if (idl_is_member(parent))
return idl_default_value(parent);
}

return NULL;
}

bool idl_is_sequence(const void *ptr)
{
idl_mask_t mask;
Expand Down Expand Up @@ -2016,6 +2029,23 @@ idl_create_enum(
ret = IDL_RETCODE_SEMANTIC_ERROR;
goto err_clash;
}
if (idl_mask(e1) == IDL_DEFAULT_ENUMERATOR) {
if (node->default_enumerator) {
idl_error(pstate, idl_location(e1),
"Assigning default to enumerator '%s' clashes with '%s' already being set as default.",
e1->name->identifier, node->default_enumerator->name->identifier);
ret = IDL_RETCODE_SEMANTIC_ERROR;
goto err_defaults;
} else {
node->default_enumerator = e1;
}
}
}

//fallback to the first entry
if (!node->default_enumerator) {
node->default_enumerator = enumerators;
node->default_enumerator->node.mask = IDL_IMPLICIT_DEFAULT_ENUMERATOR;
}

if ((ret = idl_declare(pstate, kind, name, node, NULL, NULL)))
Expand All @@ -2025,6 +2055,7 @@ idl_create_enum(
return IDL_RETCODE_OK;
err_declare:
err_clash:
err_defaults:
free(node);
err_alloc:
return ret;
Expand Down Expand Up @@ -2062,7 +2093,9 @@ static void *iterate_enumerator(const void *ptr, const void *cur)
static const char *describe_enumerator(const void *ptr)
{
(void)ptr;
assert(idl_mask(ptr) == IDL_ENUMERATOR);
assert(idl_mask(ptr) == IDL_ENUMERATOR
|| idl_mask(ptr) == IDL_DEFAULT_ENUMERATOR
|| idl_mask(ptr) == IDL_IMPLICIT_DEFAULT_ENUMERATOR);
return "enumerator";
}

Expand Down
Loading