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(compiler): Throw an error when comparing an alias and a named type with the same name [LNG-231] #946

Merged
merged 6 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
37 changes: 4 additions & 33 deletions aqua-src/antithesis.aqua
Original file line number Diff line number Diff line change
@@ -1,35 +1,6 @@
aqua A
alias Troll: u32

export bugLNG260
func test() -> i8:
MyAbility = Troll(f = "sf")

func create(a: i8) -> -> i8:
closureArrow = () -> i8:
<- a
<- closureArrow

func test() -> i8, i8:
arr1 <- create(1)
arr2 <- create(2)
<- arr1(), arr2()

func cmp(a: i32, b: i32, pred: i8 -> bool) -> bool:
result: ?bool

if a < b:
result <- pred(-1)
else:
if a == b:
result <- pred(0)
else:
result <- pred(1)

<- result!

func gt(a: i32, b: i32) -> bool:
pred = (ord: i8) -> bool:
<- ord > 0

<- cmp(a, b, pred)

func bugLNG260(a: i32, b: i32) -> bool:
<- gt(a, b)
<- 42
Original file line number Diff line number Diff line change
Expand Up @@ -618,9 +618,6 @@ class ArrowInlinerSpec extends AnyFlatSpec with Matchers with Inside {
None
)

println("testFunc: ")
println(testFunc.body.show)

val model = ArrowInliner
.callArrow[InliningState](testFunc, CallModel(Nil, Nil))
.runA(InliningState())
Expand Down
18 changes: 6 additions & 12 deletions semantics/src/main/scala/aqua/semantics/rules/ValuesAlgebra.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,21 @@ import aqua.parser.lexer.PrefixToken.Op as PrefOp
import aqua.raw.value.*
import aqua.semantics.rules.abilities.AbilitiesAlgebra
import aqua.semantics.rules.names.NamesAlgebra
import aqua.semantics.rules.types.TypesAlgebra
import aqua.semantics.rules.report.ReportAlgebra
import aqua.semantics.rules.types.TypesAlgebra
import aqua.types.*

import cats.Monad
import cats.data.OptionT
import cats.data.Chain
import cats.data.{NonEmptyList, OptionT}
import cats.instances.list.*
import cats.syntax.applicative.*
import cats.syntax.apply.*
import cats.syntax.flatMap.*
import cats.syntax.functor.*
import cats.syntax.traverse.*
import cats.syntax.foldable.*
import cats.syntax.functor.*
import cats.syntax.option.*
import cats.instances.list.*
import cats.data.{NonEmptyList, NonEmptyMap}
import cats.data.OptionT
import cats.syntax.traverse.*
import scribe.Logging

import scala.collection.immutable.SortedMap

class ValuesAlgebra[S[_], Alg[_]: Monad](using
N: NamesAlgebra[S, Alg],
T: TypesAlgebra[S, Alg],
Expand Down Expand Up @@ -119,7 +113,7 @@ class ValuesAlgebra[S[_], Alg[_]: Monad](using

case dvt @ NamedValueToken(typeName, fields) =>
(for {
resolvedType <- OptionT(T.resolveType(typeName))
resolvedType <- OptionT(T.resolveNamedType(typeName))
// Report duplicate fields
_ <- OptionT.liftF(
reportNamedArgsDuplicates(fields)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import cats.data.NonEmptyList
trait TypesAlgebra[S[_], Alg[_]] {

def resolveType(token: TypeToken[S]): Alg[Option[Type]]

def resolveNamedType(token: TypeToken[S]): Alg[Option[Type]]

def getType(name: String): Alg[Option[Type]]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ class TypesInterpreter[S[_], X](using
report.error(token, s"Unresolved type").as(None)
}

def resolveNamedType(token: TypeToken[S]): State[X, Option[Type]] =
Copy link
Contributor

@InversionSpaces InversionSpaces Oct 27, 2023

Choose a reason for hiding this comment

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

I believe result type is better to be NamedType instead of Type, or even AbilityType | StructType, to avoid pattern matching on call site

resolveType(token).flatMap(_.flatTraverse {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I believe we should move towards OptionT because we have F[Option[A]] almost all the time and it produces a lot of such flatMap(_.flatTraverse( chains. But I am not insisting on rewriting this right now

case t: (AbilityType | StructType) => Option(t).pure
case _ => report.error(token, "Type must be an ability or a data").as(None)
})

override def resolveArrowDef(arrowDef: ArrowTypeToken[S]): State[X, Option[ArrowType]] =
getState.map(TypesStateHelper.resolveArrowDef(arrowDef)).flatMap {
case Valid(TypeResolution(tt, tokens)) =>
Expand Down
51 changes: 35 additions & 16 deletions semantics/src/test/scala/aqua/semantics/ValuesAlgebraSpec.scala
Original file line number Diff line number Diff line change
@@ -1,29 +1,24 @@
package aqua.semantics

import aqua.parser.lexer.*
import aqua.raw.RawContext
import aqua.raw.value.*
import aqua.semantics.rules.ValuesAlgebra
import aqua.semantics.rules.abilities.{AbilitiesAlgebra, AbilitiesInterpreter, AbilitiesState}
import aqua.semantics.rules.names.{NamesAlgebra, NamesInterpreter, NamesState}
import aqua.semantics.rules.abilities.{AbilitiesAlgebra, AbilitiesInterpreter}
import aqua.semantics.rules.definitions.{DefinitionsAlgebra, DefinitionsInterpreter}
import aqua.semantics.rules.types.{TypesAlgebra, TypesInterpreter, TypesState}
import aqua.semantics.rules.locations.{DummyLocationsInterpreter, LocationsAlgebra}
import aqua.semantics.rules.mangler.{ManglerAlgebra, ManglerInterpreter}
import aqua.semantics.rules.names.{NamesAlgebra, NamesInterpreter, NamesState}
import aqua.semantics.rules.report.{ReportAlgebra, ReportInterpreter}
import aqua.raw.value.{ApplyBinaryOpRaw, LiteralRaw}
import aqua.raw.RawContext
import aqua.semantics.rules.types.{TypesAlgebra, TypesInterpreter}
import aqua.types.*
import aqua.parser.lexer.*
import aqua.raw.value.*
import aqua.parser.lexer.ValueToken.string

import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
import org.scalatest.Inside
import cats.Id
import cats.data.State
import cats.syntax.functor.*
import cats.syntax.comonad.*
import cats.data.NonEmptyMap
import cats.data.{NonEmptyList, NonEmptyMap, State}
import monocle.syntax.all.*
import org.scalatest.Inside
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers

import scala.collection.immutable.SortedMap

class ValuesAlgebraSpec extends AnyFlatSpec with Matchers with Inside {
Expand Down Expand Up @@ -518,6 +513,30 @@ class ValuesAlgebraSpec extends AnyFlatSpec with Matchers with Inside {
}
}

it should "throw an error when comparing a struct type with a primitive alias" in {
val state = genState(
vars = Map("SomeName" -> LiteralType.string)
)

val token = NamedValueToken[Id](
NamedTypeToken[Id]("SomeName"),
NonEmptyList.of(
NamedArg.Full(Name("f1"), LiteralToken[Id]("1", LiteralType.number)),
NamedArg.Full(Name("f2"), LiteralToken[Id]("2", LiteralType.number))
)
)

val alg = algebra()

val (st, res) = alg
.valueToRaw(token)
.run(state)
.value

res shouldBe None
st.errors.exists(_.isInstanceOf[RulesViolated[Id]]) shouldBe true
}

it should "forbid collections with abilities or arrows" in {
val ability = variable("ab")
val abilityType = AbilityType("Ab", NonEmptyMap.of("field" -> ScalarType.i8))
Expand Down
Loading