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

fix: Account for the optional equality support of map/seq inside newtypes #5980

Merged
merged 14 commits into from
Dec 17, 2024
Merged
23 changes: 6 additions & 17 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,16 @@ boogie: ${DIR}/boogie/Binaries/Boogie.exe
tests:
(cd "${DIR}"; dotnet test Source/IntegrationTests)

# make test name=<part of the path of an integration test>
# make test name=<integration test filter>
Copy link
Member

Choose a reason for hiding this comment

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

These changes to the makefile make it much much easier to test single files without having to rebuild, and also to update individual outputs without having to run the IDE or a complex command (or Lit, which we are not always supporting)

# make test name=<integration test filter> update=true to update the test
# make test name=<integration test filter> build=false don't build the solution
test:
(cd "${DIR}"; [ -z "${name}" ] && echo "Syntax: make test name=<integration test filter>" && exit 1; dotnet test Source/IntegrationTests --filter "DisplayName~${name}")
@DIR="$(DIR)" name="$(name)" update="$(update)" build="$(build)" bash scripts/test.sh

# Run Dafny on an integration test case directly in the folder itself.
# make test-run name=<part of the path> action="run ..."
# make test-dafny name=<part of the path> action="run ..." [build=false]
test-dafny:
name="$(name)"; \
files=$$(cd "${DIR}"/Source/IntegrationTests/TestFiles/LitTests/LitTest; find . -type f -wholename "*$$name*" | grep -E '\.dfy$$'); \
count=$$(echo "$$files" | wc -l); \
echo "$${files}"; \
if [ "$$count" -eq 0 ]; then \
echo "No files found matching pattern: $$name"; \
exit 1; \
else \
echo "$$count test files found."; \
for file in $$files; do \
filedir=$$(dirname "$$file"); \
(cd "${DIR}/Source/IntegrationTests/TestFiles/LitTests/LitTest/$${filedir}"; dotnet run --project "${DIR}"/Source/Dafny -- $(action) "$$(basename $$file)" ); \
done; \
fi
@name="$(name)" DIR="$(DIR)" action="$(action)" NO_BUILD=$$( [ "${build}" = "false" ] && echo "true" || echo "false" ) bash scripts/test-dafny.sh

tests-verbose:
(cd "${DIR}"; dotnet test --logger "console;verbosity=normal" Source/IntegrationTests )
Expand Down
38 changes: 38 additions & 0 deletions Scripts/test-dafny.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#!/bin/bash

# Check for required environment variables
if [ -z "$DIR" ]; then
echo "Error: DIR environment variable is not set."
exit 1
fi

if [ -z "$name" ]; then
echo "Error: name environment variable is not set."
exit 1
fi

# Set the default build flag
NO_BUILD=${NO_BUILD:-false}

# Locate files matching the specified pattern
files=$(cd "${DIR}/Source/IntegrationTests/TestFiles/LitTests/LitTest" && find . -type f -wholename "*$name*" | grep -E '\.dfy$')

if [ -z "$files" ]; then
echo "No files found matching pattern: $name"
exit 1
else
count=$(echo "$files" | wc -l)
echo "$files"
echo "$count test files found."
for file in $files; do
filedir=$(dirname "$file")
(
cd "${DIR}/Source/IntegrationTests/TestFiles/LitTests/LitTest/$filedir" || exit

build_flag=""
[ "$NO_BUILD" = "true" ] && build_flag="--no-build"

dotnet run $build_flag --project "${DIR}/Source/Dafny" -- ${action} "$(basename "$file")"
)
done
fi
59 changes: 59 additions & 0 deletions Scripts/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#!/bin/bash
set -e

# Ensure name is provided
if [ -z "$name" ]; then
echo "Syntax: make test name=<integration test filter> [update=true] [build=false]"
exit 1
fi

# Default values for update and build
update_flag=${update:-false}
build_flag=${build:-true}

# Set the integration tests folder
integration_tests_dir="${DIR}/Source/IntegrationTests"
lit_tests_dir="${integration_tests_dir}/TestFiles/LitTests/LitTest"

# Handle no-build logic
if [ "$build_flag" = "false" ]; then
echo ""
echo "Build is disabled. Copying test files to the output directory..."

framework_dir=$(find "${integration_tests_dir}/bin/Debug" -maxdepth 1 -type d -name "net*" | sort | tail -n 1)
if [ -z "$framework_dir" ]; then
echo "Error: Could not find target framework directory in bin/Debug. Please run at least once with build=true."
exit 1
fi
output_dir="${framework_dir}/TestFiles/LitTests/LitTest"

# Find and copy all matching files to the output directory
files=$(cd "$lit_tests_dir" && find . -type f -regex '.*\.\(check\|dfy\|expect\)' -wholename "*$name*")
if [ -z "$files" ]; then
echo "No files found matching pattern: $name"
exit 1
fi

# Create output directory if it doesn't exist
mkdir -p "$output_dir"

# Copy files
echo "$files" | while IFS= read -r file; do
echo "Copying $file to $output_dir"
cp "$lit_tests_dir/$file" "$output_dir/$file"
done
fi

# Check if update flag is true
if [ "$update_flag" = "true" ]; then
echo ""
echo "Update mode enabled."
echo "Going to update the .expect files to match the current Dafny output."
fi

# Run dotnet test
echo "Running integration tests..."
DAFNY_INTEGRATION_TESTS_UPDATE_EXPECT_FILE="$update_flag" \
dotnet test "$integration_tests_dir" \
$( [ "$build_flag" = "false" ] && echo "--no-build" ) \
--filter "DisplayName~$name"
18 changes: 13 additions & 5 deletions Source/DafnyCore/AST/TypeDeclarations/NewtypeDecl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,21 @@ public Type RhsWithArgument(List<Type> typeArgs) {
public TopLevelDecl AsTopLevelDecl => this;
public TypeDeclSynonymInfo SynonymInfo { get; set; }

public TypeParameter.EqualitySupportValue EqualitySupport {
private IndDatatypeDecl.ES equalitySupport = IndDatatypeDecl.ES.NotYetComputed;
Copy link
Member

Choose a reason for hiding this comment

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

The Rust compiler will also need to have precise equality support, as it's the only backend that needs to distinguish between type-parameter conditional equality and no equality possible.
Hence, it's not only solving a soundness issue, it's also making the compilation to Rust possible.


public IndDatatypeDecl.ES EqualitySupport {
get {
if (this.BaseType.SupportsEquality) {
return TypeParameter.EqualitySupportValue.Required;
} else {
return TypeParameter.EqualitySupportValue.Unspecified;
if (equalitySupport == IndDatatypeDecl.ES.NotYetComputed) {
var thingsChanged = false;
if (ModuleResolver.SurelyNeverSupportEquality(BaseType)) {
Copy link
Member

Choose a reason for hiding this comment

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

Because the types are topologically sorted, this will only be executed when the types this newtype depends on have been resolved.

equalitySupport = IndDatatypeDecl.ES.Never;
} else {
ModuleResolver.DetermineEqualitySupportType(BaseType, ref thingsChanged);
equalitySupport = IndDatatypeDecl.ES.ConsultTypeArguments;
}
}

return equalitySupport;
}
}

Expand Down
8 changes: 7 additions & 1 deletion Source/DafnyCore/AST/Types/UserDefinedType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ public override string TypeName(DafnyOptions options, ModuleDefinition context,

public override bool SupportsEquality {
get {
if (ResolvedClass is ClassLikeDecl { IsReferenceTypeDecl: true } or NewtypeDecl) {
if (ResolvedClass is ClassLikeDecl { IsReferenceTypeDecl: true }) {
return ResolvedClass.IsRevealedInScope(Type.GetScope());
} else if (ResolvedClass is TraitDecl) {
return false;
Expand All @@ -400,6 +400,12 @@ public override bool SupportsEquality {
i++;
}
return true;
} else if (ResolvedClass is NewtypeDecl newtypeDecl) {
if (newtypeDecl.IsRevealedInScope(Type.GetScope())) {
return newtypeDecl.RhsWithArgument(TypeArgs).SupportsEquality;
Copy link
Member

Choose a reason for hiding this comment

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

Because of my change, we could also do the same treatment as for the datatypes, if you preferred. I'm fine with both approaches and to switch if you prefer.

} else {
return false;
}
} else if (ResolvedClass is TypeSynonymDeclBase) {
var t = (TypeSynonymDeclBase)ResolvedClass;
if (t.SupportsEquality) {
Expand Down
31 changes: 29 additions & 2 deletions Source/DafnyCore/Resolver/CheckTypeCharacteristics_Visitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -399,13 +399,40 @@ public static bool CheckCharacteristics(TypeParameter.TypeParameterCharacteristi
return true;
}

static string TypeEqualityErrorMessageHint(Type argType) {
public static string TypeEqualityErrorMessageHint(Type argType) {
Contract.Requires(argType != null);
var cl = (argType.Normalize() as UserDefinedType)?.ResolvedClass;
argType = argType.Normalize();
var cl = (argType as UserDefinedType)?.ResolvedClass;
var tp = (TopLevelDecl)(cl as TypeParameter) ?? cl as AbstractTypeDecl;
if (tp != null) {
return string.Format(" (perhaps try declaring {2} '{0}' on line {1} as '{0}(==)', which says it can only be instantiated with a type that supports equality)", tp.Name, tp.tok.line, tp.WhatKind);
}

var typeArgs = argType.TypeArgs;

if (argType.AsSeqType != null && typeArgs.Count >= 1) {
if (TypeEqualityErrorMessageHint(typeArgs[0]) is var messageSeq and not "") {
return messageSeq;
}
}
if (argType.AsMapType != null &&
typeArgs.Count >= 2 &&
TypeEqualityErrorMessageHint(typeArgs[1]) is var messageMap and not "") {
return messageMap;
}
if (argType.AsIndDatatype is { EqualitySupport: IndDatatypeDecl.ES.ConsultTypeArguments } decl) {
var i = 0;
foreach (var tParam in decl.TypeArgs) {
if (tParam.NecessaryForEqualitySupportOfSurroundingInductiveDatatype && i < typeArgs.Count && !typeArgs[i].SupportsEquality && TypeEqualityErrorMessageHint(typeArgs[i]) is var message and not "") {
return message;
}
i++;
}
}

if (argType.AsNewtype is { } newTypeDecl) {
return TypeEqualityErrorMessageHint(newTypeDecl.RhsWithArgument(argType.TypeArgs));
}
return "";
}
}
88 changes: 64 additions & 24 deletions Source/DafnyCore/Resolver/ModuleResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2861,11 +2861,7 @@ void MarkSCCAsNotSupportingEquality() {
return; // we are done
}
foreach (var arg in ctor.Formals) {
var anotherIndDt = arg.Type.AsIndDatatype;
if (arg.IsGhost ||
(anotherIndDt != null && anotherIndDt.EqualitySupport == IndDatatypeDecl.ES.Never) ||
arg.Type.IsCoDatatype ||
arg.Type.IsArrowType) {
if (arg.IsGhost || SurelyNeverSupportEquality(arg.Type)) {
Copy link
Member

Choose a reason for hiding this comment

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

We need this type traversal to be recursive, otherwise soundness issue 5972c.dfy

// arg.Type is known never to support equality
MarkSCCAsNotSupportingEquality();
return; // we are done
Expand All @@ -2884,8 +2880,7 @@ void MarkSCCAsNotSupportingEquality() {
}
foreach (var ctor in dt.Ctors) {
foreach (var arg in ctor.Formals) {
var type = arg.Type;
DetermineEqualitySupportType(type, ref thingsChanged);
DetermineEqualitySupportType(arg.Type, ref thingsChanged);
}
}
}
Expand Down Expand Up @@ -2925,30 +2920,75 @@ void MarkSCCAsNotSupportingEquality() {
}
}

private static void DetermineEqualitySupportType(Type type, ref bool thingsChanged) {
public static bool SurelyNeverSupportEqualityTypeParameters(IndDatatypeDecl.ES equalitySupport, List<TypeParameter> typeParams, List<Type> typeArgs) {
return
equalitySupport == IndDatatypeDecl.ES.Never ||
(equalitySupport == IndDatatypeDecl.ES.ConsultTypeArguments &&
typeArgs.Zip(typeParams).Any(tt =>
tt.Item2.NecessaryForEqualitySupportOfSurroundingInductiveDatatype && SurelyNeverSupportEquality(tt.Item1)));
}

// If returns true, the given type never supports equality
// If return false, then the type must support equality if type parameters support equality
// It is unsound for a type to make this function return false when there is no type parameter
// assignment that makes this type support equality
public static bool SurelyNeverSupportEquality(Type type) {
type = type.NormalizeExpand();
return
type.AsNewtype is { EqualitySupport: var equalitySupport, TypeArgs: var typeParams }
&& SurelyNeverSupportEqualityTypeParameters(equalitySupport, typeParams, type.TypeArgs)
||
type.AsIndDatatype is { EqualitySupport: var equalitySupport2, TypeArgs: var typeParams2 }
&& SurelyNeverSupportEqualityTypeParameters(equalitySupport2, typeParams2, type.TypeArgs)
||
type.IsCoDatatype || type.IsArrowType ||
type.AsSeqType is { Arg: var argType } && SurelyNeverSupportEquality(argType) ||
type.AsMapType is { Range: var rangeType } && SurelyNeverSupportEquality(rangeType);
}

public static void DetermineEqualitySupportType(Type type, ref bool thingsChanged) {
if (type.AsTypeParameter is { } typeArg) {
typeArg.NecessaryForEqualitySupportOfSurroundingInductiveDatatype = true;
thingsChanged = true;
} else if (type.AsMapType is { } typeMap) {
DetermineEqualitySupportType(typeMap.Range, ref thingsChanged);
} else if (type.AsSeqType is { } typeSeq) {
DetermineEqualitySupportType(typeSeq.Arg, ref thingsChanged);
} else if (type.AsIndDatatype is { } otherDt) {
if (otherDt.EqualitySupport == IndDatatypeDecl.ES.ConsultTypeArguments) { // datatype is in a different SCC
var otherUdt = (UserDefinedType)type.NormalizeExpand();
var i = 0;
foreach (var otherTp in otherDt.TypeArgs) {
if (otherTp.NecessaryForEqualitySupportOfSurroundingInductiveDatatype) {
var tp = otherUdt.TypeArgs[i].AsTypeParameter;
if (tp != null) {
tp.NecessaryForEqualitySupportOfSurroundingInductiveDatatype = true;
thingsChanged = true;
}
} else if (type.Normalize() is UserDefinedType userDefinedType) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this normalization? I thought that AsMapType and AsSeqType would actually do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This type.Normalize() cuts through any type proxies that may be sitting around the UserDefinedType.

AsMapType and AsSeqType call NormalizeExpand(), which cuts through both proxies and type synonyms. But that's too much, because a type synonym can also have an explicit (==) declaration.

Copy link
Member

Choose a reason for hiding this comment

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

If a type synonym does not have an explicit (==) annotation, it's still sound, right?
In general, I don't think you can conclude faster that a type synonym supports equality anyway. It seems that (==) is only a way to catch errors earlier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An explicit (==) is also a way to advertise equality support to importers of the enclosing module. Without an explicit (==), an export provides of the type would not let importers use the type as supporting equality.

if (userDefinedType.ResolvedClass is TypeSynonymDeclBase typeSynonymDecl) {
if (typeSynonymDecl.IsRevealedInScope(Type.GetScope())) {
if (typeSynonymDecl.Characteristics.EqualitySupport == TypeParameter.EqualitySupportValue.Required) {
return; // It's guaranteed that this type synonym requires equality
}
DetermineEqualitySupportType(typeSynonymDecl.RhsWithArgument(userDefinedType.TypeArgs), ref thingsChanged);
}
} else if (userDefinedType.ResolvedClass is NewtypeDecl newtypeDecl) {
if (newtypeDecl.IsRevealedInScope(Type.GetScope())) {
DetermineEqualitySupportType(newtypeDecl.RhsWithArgument(userDefinedType.TypeArgs), ref thingsChanged);
}
} else if (userDefinedType.ResolvedClass is IndDatatypeDecl otherDt) {
if (otherDt.EqualitySupport == IndDatatypeDecl.ES.ConsultTypeArguments) {
// datatype is in a different SCC
var otherUdt = (UserDefinedType)type.NormalizeExpand();
var i = 0;
foreach (var otherTp in otherDt.TypeArgs) {
if (otherTp.NecessaryForEqualitySupportOfSurroundingInductiveDatatype) {
var tp = otherUdt.TypeArgs[i].AsTypeParameter;
if (tp != null) {
tp.NecessaryForEqualitySupportOfSurroundingInductiveDatatype = true;
thingsChanged = true;
}
}

i++;
i++;
}
}
}
} else if (type.AsMapType is { } typeMap) {
// A map type's Domain type is required to support equality (just like the argument type for sets and multisets), but
// it is optional for the map type's Range type to support equality. Thus, we need to determine the equality support
// for just the Range type.
DetermineEqualitySupportType(typeMap.Range, ref thingsChanged);
} else if (type.AsSeqType is { } typeSeq) {
// Like the Range type of a map, it is optional for a sequence type's argument type to support equality. So, we make a call
// to determine it.
DetermineEqualitySupportType(typeSeq.Arg, ref thingsChanged);
}
}

Expand Down
4 changes: 2 additions & 2 deletions Source/DafnyCore/Resolver/TypeCharacteristicChecker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ private static void Check(List<TopLevelDecl> declarations, bool isAnExport, Erro
visitor.VisitType(syn.tok, syn.Rhs, false);
if (!isAnExport) {
if (syn.SupportsEquality && !syn.Rhs.SupportsEquality) {
reporter.Error(MessageSource.Resolver, syn.tok, "type '{0}' declared as supporting equality, but the RHS type ({1}) might not",
syn.Name, syn.Rhs);
reporter.Error(MessageSource.Resolver, syn.tok, "type '{0}' declared as supporting equality, but the RHS type ({1}) might not{2}",
syn.Name, syn.Rhs, CheckTypeCharacteristics_Visitor.TypeEqualityErrorMessageHint(syn.Rhs));
}
if (syn.Characteristics.IsNonempty && !syn.Rhs.IsNonempty) {
reporter.Error(MessageSource.Resolver, syn.tok, "type '{0}' declared as being nonempty, but the RHS type ({1}) may be empty",
Expand Down
Loading
Loading