diff --git a/vadl/main/vadl/ast/TypeChecker.java b/vadl/main/vadl/ast/TypeChecker.java index 52ac8fdf7..9f26153cf 100644 --- a/vadl/main/vadl/ast/TypeChecker.java +++ b/vadl/main/vadl/ast/TypeChecker.java @@ -20,6 +20,7 @@ import static vadl.error.Diagnostic.error; import static vadl.error.Diagnostic.warning; +import com.google.common.collect.Streams; import java.math.BigInteger; import java.util.ArrayDeque; import java.util.ArrayList; @@ -545,6 +546,37 @@ private static Expr wrapImplicitCast(Expr inner, Type to) { return new CastExpr(inner, to); } + /** + * Wraps the expr provided with an implicit cast if it is possible, and not useless. + * + * @param inner expression to wrap. + * @param to which the expression should be casted. + * @return the original expression, possibly wrapped. + */ + private static Expr wrapImplicitCastConstToTypeClass(Expr inner, Class to) { + var innerType = requireNonNull(inner.type); + + // Only for const types + if (!(innerType instanceof ConstantType innerConstType)) { + return inner; + } + + if (to == BoolType.class) { + return wrapImplicitCast(inner, Type.bool()); + } + if (to == SIntType.class) { + return wrapImplicitCast(inner, innerConstType.closestSInt()); + } + if (to == UIntType.class) { + return wrapImplicitCast(inner, innerConstType.closestUInt()); + } + if (to == BitsType.class) { + return wrapImplicitCast(inner, innerConstType.closestBits()); + } + + return inner; + } + /** * Wraps the expr provided with an explicit cast if it is possible, and not useless. * However, in comparison to {@link #wrapExplicitCast(Expr, Type)}, this will throw a @@ -614,6 +646,15 @@ private BuiltInCheckResult checkBuiltin(BuiltInTable.BuiltIn builtIn, List // Check all incoming arguments args.forEach(this::check); + if (!(args.size() == builtIn.argTypeClasses().size() || (builtIn.signature().hasVarArgs() + && args.size() >= builtIn.argTypeClasses().size()))) { + throw addErrorAndStopChecking( + error("Type Mismatch", location) + .locationDescription(location, + "Expected %d arguments but got %d.", builtIn.argTypeClasses().size(), args.size()) + .build()); + } + if (args.size() == 1) { var innerType = args.getFirst().type(); @@ -824,23 +865,45 @@ private BuiltInCheckResult checkBuiltin(BuiltInTable.BuiltIn builtIn, List } var argTypes = args.stream().map(Expr::type).toList(); - var areAllArgs = argTypes.stream().allMatch(ConstantType.class::isInstance); - if (areAllArgs) { + var areAllConst = argTypes.stream().allMatch(ConstantType.class::isInstance); + if (areAllConst) { var type = constantEvaluator .evalBuiltin(builtIn, args.stream().map(constantEvaluator::eval).toList(), location) .type(); return new BuiltInCheckResult(type, null); } + // There are vararg functions so let's assume the last type is used for all additional args like + // in Java. + var declaredTypes = builtIn.signature().hasVarArgs() + ? Streams.concat( + builtIn.argTypeClasses().stream(), + Stream.generate(() -> builtIn.argTypeClasses().getLast())) + : builtIn.argTypeClasses().stream(); + + // Inject implicit casts for constant types + // NOTE: There might be functions that operate on bit patterns where this implicit cast might + // not be intended and should be disallowed. + args = Streams.zip(args.stream(), declaredTypes, TypeChecker::wrapImplicitCastConstToTypeClass) + .toList(); + var originalArgTypes = argTypes; + argTypes = args.stream().map(Expr::type).toList(); + + if (!builtIn.takes(argTypes)) { - // FIXME: Better format that error - addErrorAndStopChecking(error("Type Mismatch", location) - .description("Expected %s but got `%s`", builtIn.signature().argTypeClasses(), argTypes) + // FIXME: Further improve these error messages. + var areSomeConst = originalArgTypes.stream().anyMatch(ConstantType.class::isInstance); + var calledTypes = String.join(", ", argTypes.stream().map(Type::toString).toList()); + addErrorAndStopChecking( + error("Type Mismatch", location) + .locationDescription(location, "The builtin has the signature `%s` but got `%s`.", + builtIn.signature(), calledTypes) + .applyIf(areSomeConst, b -> b.locationHelp(location, + "Try casting some of the constant arguments to explicit types.")) .build()); } - // Note: cannot set the computed type because builtins aren't a definition. - return new BuiltInCheckResult(builtIn.returns(argTypes), null); + return new BuiltInCheckResult(builtIn.returns(argTypes), args); } @Override @@ -1717,8 +1780,15 @@ public Void visit(GroupDefinition groupDefinition) { public Void visit(ApplicationBinaryInterfaceDefinition definition) { definition.definitions.forEach(this::check); + // NOTE: The keys are sorted such that the test always produce the same errors if there + // are multiple errors. + var keys = SpecialPurposeRegisterDefinition.Purpose.numberOfOccurrencesAbi.entrySet() + .stream() + .sorted(Map.Entry.comparingByKey()) + .toList(); + // Check the number of occurrences in the ABI. - for (var entry : SpecialPurposeRegisterDefinition.Purpose.numberOfOccurrencesAbi.entrySet()) { + for (var entry : keys) { var purpose = entry.getKey(); var registers = definition.definitions.stream().filter( x -> x instanceof SpecialPurposeRegisterDefinition specialPurposeRegisterDefinition @@ -2675,7 +2745,7 @@ private void visitIdentifiable(Expr expr) { // it returns the type of the function. This is only possible because we don't have // function overloading. errors.add(error("Invalid Function Call", expr) - .description("Expected `%s` arguments but got `%s`", functionDefinition.params.size(), + .description("Expected `%s` arguments but got `%s`.", functionDefinition.params.size(), 0) .build()); } @@ -2691,7 +2761,7 @@ private void visitIdentifiable(Expr expr) { if (!exceptionDef.params.isEmpty()) { // No need to stop evaluation we can still continue. errors.add(error("Invalid Exception Raise", expr) - .description("Expected `%s` arguments but got `%s`", exceptionDef.params.size(), + .description("Expected %s arguments but got %s.", exceptionDef.params.size(), 0) .build()); } @@ -4214,7 +4284,7 @@ public Void visit(InstructionCallStatement statement) { var argCount = statement.unnamedArguments.size(); if (paramCount != argCount) { addErrorAndStopChecking(error("Arguments Mismatch", statement.location()) - .description("Expected %s arguments but got %s", paramCount, argCount) + .description("Expected %s arguments but got %s.", paramCount, argCount) .build()); } diff --git a/vadl/main/vadl/error/DiagnosticBuilder.java b/vadl/main/vadl/error/DiagnosticBuilder.java index facc8d4b2..2d814db67 100644 --- a/vadl/main/vadl/error/DiagnosticBuilder.java +++ b/vadl/main/vadl/error/DiagnosticBuilder.java @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText : © 2025 TU Wien +// SPDX-FileCopyrightText : © 2025-2026 TU Wien // SPDX-License-Identifier: GPL-3.0-or-later // // This program is free software: you can redistribute it and/or modify @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; import vadl.utils.SourceLocation; import vadl.utils.WithLocation; @@ -175,6 +176,24 @@ public DiagnosticBuilder suggestions(List items) { return this.help("Did you maybe mean one of: %s", text); } + /** + * A conditional application of further information to a diagnostic. + * The same can be archived by storring the builder in a variable and applying some items only + * conditionally but this interface is easier to read. + * + * @param condition that determines whether the function is applied or not. + * @param function to be applied if the condition is true. + * @return the builder itself. + */ + public DiagnosticBuilder applyIf(boolean condition, + Function function) { + if (condition) { + return function.apply(this); + } else { + return this; + } + } + public Diagnostic build() { return diagnostic; } diff --git a/vadl/test/resources/frontend-snapshots/typechecker/assemblyBuiltinWithoutCast.vadl b/vadl/test/resources/frontend-snapshots/typechecker/assemblyBuiltinWithoutCast.vadl new file mode 100644 index 000000000..bb2ebd240 --- /dev/null +++ b/vadl/test/resources/frontend-snapshots/typechecker/assemblyBuiltinWithoutCast.vadl @@ -0,0 +1,27 @@ +// Previously, this also threw an type error. +// https://github.com/OpenVADL/openvadl/issues/787 + +instruction set architecture ISA = {} + +// NOTE: This one error for the empty ABI cannot be avoided in an empty ISA spec. +application binary interface ABI for ISA = {} + +assembly description AD for ABI = { + grammar = { + Rule1 : ?( LaIdEq(1, "a") ) "a" | "a" ; + // ^^^^^^^^^^^^^^ Previously, this threw an error. + } +} + + +// Reported Diagnostics: +// +// error: No RETURN_ADDRESS registers were declared but one was expected +// ╭──[test/resources/frontend-snapshots/typechecker/assemblyBuiltinWithoutCast.vadl:7:1] +// │ +// 7 │ application binary interface ABI for ISA = {} +// │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +// │ +// +// +// Part of the class vadl.ast.FrontendSnapshotTests \ No newline at end of file diff --git a/vadl/test/resources/frontend-snapshots/typechecker/invalidBuiltitinCalls.vadl b/vadl/test/resources/frontend-snapshots/typechecker/invalidBuiltitinCalls.vadl new file mode 100644 index 000000000..5e9cee2d0 --- /dev/null +++ b/vadl/test/resources/frontend-snapshots/typechecker/invalidBuiltitinCalls.vadl @@ -0,0 +1,36 @@ +// Call with incorrect number of arguments +constant a = VADL::add(1) +constant b = VADL::add(1, 2, 3) + +// Function call with one constant and one variable +// FIXME: In the future there will be special rules for casting here so a better builtin has to be found, because this +// should work. +function abc(x: Bits<8>) -> Bits<8> = VADL::adds(1, x) + + +// Reported Diagnostics: +// +// error: Type Mismatch +// ╭──[test/resources/frontend-snapshots/typechecker/invalidBuiltitinCalls.vadl:2:14] +// │ +// 2 │ constant a = VADL::add(1) +// │ ^^^^^^^^^^^^ Expected 2 arguments but got 1. +// │ +// +// error: Type Mismatch +// ╭──[test/resources/frontend-snapshots/typechecker/invalidBuiltitinCalls.vadl:3:14] +// │ +// 3 │ constant b = VADL::add(1, 2, 3) +// │ ^^^^^^^^^^^^^^^^^^ Expected 2 arguments but got 3. +// │ +// +// error: Type Mismatch +// ╭──[test/resources/frontend-snapshots/typechecker/invalidBuiltitinCalls.vadl:8:39] +// │ +// 8 │ function abc(x: Bits<8>) -> Bits<8> = VADL::adds(1, x) +// │ ^^^^^^^^^^^^^^^^ The builtin has the signature `(BitsType, BitsType) -> TupleType` but got `Bits<1>, Bits<8>`. +// │ help: Try casting some of the constant arguments to explicit types. +// │ +// +// +// Part of the class vadl.ast.FrontendSnapshotTests \ No newline at end of file diff --git a/vadl/test/vadl/ast/AsmLL1CheckerTest.java b/vadl/test/vadl/ast/AsmLL1CheckerTest.java index 76dbcdf81..a7f295d49 100644 --- a/vadl/test/vadl/ast/AsmLL1CheckerTest.java +++ b/vadl/test/vadl/ast/AsmLL1CheckerTest.java @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText : © 2025 TU Wien +// SPDX-FileCopyrightText : © 2025-2026 TU Wien // SPDX-License-Identifier: GPL-3.0-or-later // // This program is free software: you can redistribute it and/or modify @@ -500,8 +500,7 @@ void conflictWithTerminal() { Assertions.assertEquals(1, diags.items.size()); } - // FIXME: re-enable when parameters of asm built-in functions are correctly casted - // @Test + @Test void asmBuiltInUsage() { var prog = """ grammar = { @@ -530,8 +529,7 @@ void conflictInExpandedInstructionRule() { Assertions.assertEquals(1, diags.items.size()); } - // FIXME: re-enable when parameters of asm built-in functions are correctly casted - // @Test + @Test void conflictInExpandedInstructionRuleResolvedByRewriting() { var prog = """ grammar = {