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

Warn on unchecked type constraints #1066

Merged
merged 1 commit into from
Feb 16, 2024
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
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
}
}
]
}
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
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
}
}
]
}
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
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
Loading