Skip to content

Commit

Permalink
Warn on unchecked type constraints
Browse files Browse the repository at this point in the history
Bugs fixed:
- Give a warning when referencing generic types or functions that have constraints on their type arguments (closes #1065)

Notes:
- This *does not* prevent generation of potentially incorrect and unsafe IL. It simply issues a warning where types or functions with type constraints are referenced. See #115 for details.
  • Loading branch information
degory committed Feb 16, 2024
1 parent 58d106f commit 8a92ac7
Show file tree
Hide file tree
Showing 24 changed files with 186 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .config/dotnet-tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
]
},
"ghul.compiler": {
"version": "0.7.5",
"version": "0.7.6",
"commands": [
"ghul-compiler"
]
Expand Down
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project>
<PropertyGroup>
<Version>0.7.6-alpha.4</Version>
<Version>0.7.7-alpha.25</Version>
<NoWarn>$(NoWarn);NU1507</NoWarn>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>

<GhulCompiler>dotnet ghul-compiler</GhulCompiler>
</PropertyGroup>

<ItemGroup>
<GhulSources Include="**/*.ghul" />
<PackageReference Include="ghul.runtime" />
<PackageReference Include="ghul.pipes" />
<PackageReference Include="ghul.targets" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"version": "2.0.0",
"tasks": [
{
"label": "Run test",
"command": "dotnet ghul-test \"${workspaceFolder}\"",
"type": "shell",
"group": {
"kind": "test",
"isDefault": true
}
},
{
"label": "Capture test expectation",
"command": "../../../tasks/capture.sh \"${workspaceFolder}\"",
"type": "shell",
"group": {
"kind": "build",
"isDefault": true
}
}
]
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"compiler": "dotnet ../../../publish/ghul.dll",
"source": [
"."
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--type-check
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace Test.GenericArguments is
entry() is
let p: System.IParsable`1[int];
si
si
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test.ghul: 3,16..3,39: warn: type System.IParsable`1[Ghul.int] has unchecked constraints
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"version": "2.0.0",
"tasks": [
{
"label": "Run test",
"command": "dotnet ghul-test \"${workspaceFolder}\"",
"type": "shell",
"group": {
"kind": "test",
"isDefault": true
}
},
{
"label": "Capture test expectation",
"command": "../../../tasks/capture.sh \"${workspaceFolder}\"",
"type": "shell",
"group": {
"kind": "build",
"isDefault": true
}
}
]
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"compiler": "dotnet ../../../publish/ghul.dll",
"source": [
"."
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--type-check
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace Test.GenericArguments is
entry() is
double.create_checked`[int](123);
si
si
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test.ghul: 3,9..3,41: warn: call to Ghul.double.create_checked[Ghul.int](value: int) -> Ghul.double has unchecked constraints
88 changes: 85 additions & 3 deletions src/semantic/dotnet/symbol_factory.ghul
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ namespace Semantic.DotNet is
use System.Reflection.BindingFlags;
use System.Reflection.MemberTypes;

use System.Reflection.GenericParameterAttributes;

use Collections.List;
use Collections.MutableList;
use Collections.LIST;
Expand Down Expand Up @@ -77,6 +79,9 @@ namespace Semantic.DotNet is
"System.ValueTuple`6",
"System.ValueTuple`7"
]);

// TODO handle more than 7-tuple types by
// nesting them
si

set_symbol_table(symbol_table: SYMBOL_TABLE) is
Expand Down Expand Up @@ -105,7 +110,6 @@ namespace Semantic.DotNet is
let il_name = type_details.il_name;

let owner = new EMPTY_SCOPE(type_details.ghul_namespace);
// _namespaces.find_or_add_namespace(namespace_name.name, "." + namespace_name.qualified_name);

let arguments = new LIST[string]();

Expand All @@ -126,9 +130,11 @@ namespace Semantic.DotNet is
result.mark_overrides_resolved();
elif dotnet_type.is_class then
let result_class = new Symbols.CLASS(Source.LOCATION.reflected, Source.LOCATION.reflected, owner, ghul_type_name, arguments, true, owner);

if dotnet_type == _object_type then
result_class.mark_is_object();
fi

result = result_class;
result.mark_overrides_resolved();
elif dotnet_type.is_value_type then
Expand All @@ -139,7 +145,7 @@ namespace Semantic.DotNet is
result = new Symbols.STRUCT(Source.LOCATION.reflected, Source.LOCATION.reflected, owner, ghul_type_name, arguments, true, owner);
result.il_is_primitive_type = dotnet_type.is_primitive \/ dotnet_type == _void_type;
fi

result.mark_overrides_resolved();
elif dotnet_type.is_interface then
result = new Symbols.TRAIT(Source.LOCATION.reflected, Source.LOCATION.reflected, owner, ghul_type_name, arguments, true, owner);
Expand Down Expand Up @@ -175,10 +181,82 @@ namespace Semantic.DotNet is
result.add_ancestor(_type_mapper.get_type(i));
od

check_constraints_for_type(result, type);

// FIXME: why is this needed if we've already marked overrides resolved?
IoC.CONTAINER.instance.symbol_table.enter_scope(result);
result.pull_down_super_symbols();
IoC.CONTAINER.instance.symbol_table.leave_scope(result);
si

check_constraints_for_type(symbol: Symbols.Scoped, type: TYPE) is
if
type.is_generic_type /\
type.contains_generic_parameters /\
type.get_generic_arguments() |
.has(t =>
t.is_generic_type_parameter /\
!are_generic_parameter_constraints_safe(t)
)
then
IO.Std.error.write_line("type has unsafe generic parameter constraints: " + symbol.name);
symbol.is_unsafe_constraints = true;
fi
si

check_constraints_for_method(owner: Symbols.Symbol, symbol: Symbols.Scoped, generic_arguments: Collections.Iterable[TYPE]) is
if generic_arguments | .has(t => !are_generic_parameter_constraints_safe(t)) then
IO.Std.error.write_line("method has unsafe generic parameter constraints: " + owner.name + "." + symbol.name);
symbol.is_unsafe_constraints = true;
fi
si

mask_gpa(a: GenericParameterAttributes, b: GenericParameterAttributes) -> GenericParameterAttributes =>
cast GenericParameterAttributes(cast int(a) ∩ cast int(b));

are_generic_parameter_constraints_safe(t: TYPE) -> bool is
let constraint_attributes = mask_gpa(t.generic_parameter_attributes, GenericParameterAttributes.SPECIAL_CONSTRAINT_MASK);

if constraint_attributes != GenericParameterAttributes.NONE then
// we can't handle this constraint so the type or method is unsafe
return false;
fi

return t.get_generic_parameter_constraints().count == 0;
si

/*
list_generic_parameter_attributes(t: TYPE) -> string is
let gpa = t.generic_parameter_attributes;
let variance = mask_gpa(gpa, GenericParameterAttributes.VARIANCE_MASK);

let result = new LIST[string](2);

if variance != GenericParameterAttributes.NONE then
if mask_gpa(variance, GenericParameterAttributes.COVARIANT) != GenericParameterAttributes.NONE then
result.add("Covariant");
else
result.add("Contravariant");
fi
fi

let constraints = mask_gpa(gpa, GenericParameterAttributes.SPECIAL_CONSTRAINT_MASK);

if constraints != GenericParameterAttributes.NONE then
if mask_gpa(gpa, GenericParameterAttributes.REFERENCE_TYPE_CONSTRAINT) != GenericParameterAttributes.NONE then
result.add(" ReferenceTypeConstraint");
fi
if mask_gpa(gpa, GenericParameterAttributes.NOT_NULLABLE_VALUE_TYPE_CONSTRAINT) != GenericParameterAttributes.NONE then
result.add(" NotNullableValueTypeConstraint");
fi
if mask_gpa(gpa, GenericParameterAttributes.DEFAULT_CONSTRUCTOR_CONSTRAINT) != GenericParameterAttributes.NONE then
result.add(" DefaultConstructorConstraint");
fi
fi

return result | .to_string(" ");
si
*/

add_members(result: Symbols.Scoped, type: TYPE) is
let properties = new MAP[System.Reflection.MethodInfo,PROPERTY_DETAILS]();
Expand Down Expand Up @@ -453,10 +531,14 @@ namespace Semantic.DotNet is
let generic_argument_names = new LIST[string]();
let generic_arguments = new LIST[Type]();

for p in method.get_generic_arguments() do
let ga = method.get_generic_arguments();

for p in ga do
generic_argument_names.add(p.name);
generic_arguments.add(_type_mapper.get_type(p));
od

check_constraints_for_method(owner, result, ga);

result.generic_argument_names = generic_argument_names;
result.generic_arguments = generic_arguments;
Expand Down
2 changes: 0 additions & 2 deletions src/semantic/symbols/generic.ghul
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ namespace Semantic.Symbols is
short_description: string is
let result = new System.Text.StringBuilder();

result.append("&SSD&");

result
.append(name)
.append('[');
Expand Down
2 changes: 2 additions & 0 deletions src/semantic/symbols/symbol.ghul
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ namespace Semantic.Symbols is

// FIXME: can these go somewhere more specific?
il_is_primitive_type: bool public;

is_unsafe_constraints: bool public;

argument_names: Collections.List[string] => new Collections.LIST[string](0);
arguments: Collections.List[Type] => new Collections.LIST[Type](0);
Expand Down
18 changes: 17 additions & 1 deletion src/syntax/process/compile_expressions.ghul
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,10 @@ namespace Syntax.Process is

let function = overload_result.function;

if function.is_unsafe_constraints then
_logger.warn(unary.location, "call to " + function + " has unchecked constraints");
fi

_symbol_use_locations.add_symbol_use(unary.operation.location, function);

let value = function.call(unary.location, null, new Collections.LIST[Value]([unary.right.value]), null, _function_caller);
Expand Down Expand Up @@ -1170,6 +1174,10 @@ namespace Syntax.Process is
fi

let value: Value;

if function.is_unsafe_constraints then
_logger.warn(binary.location, "call to " + function + " has unchecked constraints");
fi

if function.is_instance then
value = function.call(binary.location, binary.left.value, new Collections.LIST[Value]([binary.right.value]), force_type,_function_caller);
Expand Down Expand Up @@ -1311,8 +1319,12 @@ namespace Syntax.Process is
index.left.location.end_column+1,
index.index.location.start_line,
index.index.location.end_column-1
), function);
), function);

if function.is_unsafe_constraints then
_logger.warn(index.location, "call to " + function + " has unchecked constraints");
fi

if need_store then
index.value =
new TYPE_WRAPPER(
Expand Down Expand Up @@ -1822,6 +1834,10 @@ namespace Syntax.Process is

let function = overload_result.function;

if function.is_unsafe_constraints then
_logger.warn(call.location, "call to " + function + " has unchecked constraints");
fi

_symbol_use_locations.add_symbol_use(call.function.right_location, function);

call.value = function.call(call.function.location, load.from, arguments, null, _function_caller);
Expand Down
4 changes: 4 additions & 0 deletions src/syntax/process/resolve_type_expressions.ghul
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ namespace Syntax.Process is
generic.location,
cast Semantic.Symbols.Classy(symbol),
arguments);

if symbol.is_unsafe_constraints then
_logger.warn(generic.location, "type " + generic.type + " has unchecked constraints");
fi
fi
si

Expand Down

0 comments on commit 8a92ac7

Please sign in to comment.