Skip to content
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
92 changes: 81 additions & 11 deletions vadl/main/vadl/ast/TypeChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<? extends Type> 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
Expand Down Expand Up @@ -614,6 +646,15 @@ private BuiltInCheckResult checkBuiltin(BuiltInTable.BuiltIn builtIn, List<Expr>
// 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();

Expand Down Expand Up @@ -824,23 +865,45 @@ private BuiltInCheckResult checkBuiltin(BuiltInTable.BuiltIn builtIn, List<Expr>
}

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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
}
Expand All @@ -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());
}
Expand Down Expand Up @@ -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());
}

Expand Down
21 changes: 20 additions & 1 deletion vadl/main/vadl/error/DiagnosticBuilder.java
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// SPDX-FileCopyrightText : © 2025 TU Wien <vadl@tuwien.ac.at>
// SPDX-FileCopyrightText : © 2025-2026 TU Wien <vadl@tuwien.ac.at>
// SPDX-License-Identifier: GPL-3.0-or-later
//
// This program is free software: you can redistribute it and/or modify
Expand All @@ -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;
Expand Down Expand Up @@ -175,6 +176,24 @@ public DiagnosticBuilder suggestions(List<String> 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<DiagnosticBuilder, DiagnosticBuilder> function) {
if (condition) {
return function.apply(this);
} else {
return this;
}
}

public Diagnostic build() {
return diagnostic;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
8 changes: 3 additions & 5 deletions vadl/test/vadl/ast/AsmLL1CheckerTest.java
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// SPDX-FileCopyrightText : © 2025 TU Wien <vadl@tuwien.ac.at>
// SPDX-FileCopyrightText : © 2025-2026 TU Wien <vadl@tuwien.ac.at>
// SPDX-License-Identifier: GPL-3.0-or-later
//
// This program is free software: you can redistribute it and/or modify
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down
Loading