Skip to content

Commit

Permalink
Fix type constraint check against NullablePointer being ommitted in F…
Browse files Browse the repository at this point in the history
…FI declarations

This commit makes the expr pre check for use commands more fine-grained:
we're only interested in ignoring the "guard" part of a use command, but
we still want to go through the arguments to check for any possible
errors during the expr pass. In particular, we're interested in the
NullablePointer constraint check.
  • Loading branch information
ergl authored and SeanTAllen committed May 21, 2021
1 parent 3bdad0d commit 5e676cf
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .release-notes/3758.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## Fix type constraint check against NullablePointer being omitted in FFI declarations

This release fixes an issue where the compiler would not perform some type checks against FFI declarations. Specifically, the compiler would fail to validate that the type parameter to the `NullablePointer` type had to be a struct type. This check is important since a program that used a non-struct type in a `NullablePointer` could theoretically segfault at runtime.
12 changes: 8 additions & 4 deletions src/libponyc/pass/expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,14 @@ ast_result_t pass_pre_expr(ast_t** astp, pass_opt_t* options)
switch(ast_id(ast))
{
case TK_ARRAY: return expr_pre_array(options, astp);
case TK_USE:
// Don't look in use commands to avoid false type errors from the guard
return AST_IGNORE;

case TK_IFDEFNOT:
case TK_IFDEFAND:
case TK_IFDEFOR:
case TK_IFDEFFLAG:
// Don't look in guards for use commands to avoid false type errors
if((ast_parent(ast) != NULL) && (ast_id(ast_parent(ast)) == TK_USE))
return AST_IGNORE;
break;
default: {}
}

Expand Down
26 changes: 26 additions & 0 deletions test/libponyc/ffi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
// FFI type checking tests

#define TEST_ERROR(src) DO(test_error(src, "expr"))

#define TEST_ERRORS_2(src, err1, err2) \
{ const char* errs[] = {err1, err2, NULL}; \
DO(test_expected_errors(src, "expr", errs)); }

#define TEST_COMPILE(src) DO(test_compile(src, "expr"))


Expand Down Expand Up @@ -201,3 +206,24 @@ TEST_F(FFITest, DeclarationAlias)

TEST_ERROR(src);
}

TEST_F(FFITest, DeclarationWithNullablePointer)
{
// From issue #3757
const char* src =
"use @foo[NullablePointer[(U64, U64)]]()\n"
"use @bar[NullablePointer[P1]]()\n"
"primitive P1\n"

"actor Main\n"
" new create(env: Env) =>\n"
" None";

TEST_ERRORS_2(src,
"NullablePointer[(U64 val, U64 val)] ref is not allowed: "
"the type argument to NullablePointer must be a struct",

"NullablePointer[P1 val] ref is not allowed: "
"the type argument to NullablePointer must be a struct");

}

0 comments on commit 5e676cf

Please sign in to comment.