Defining built-ins in experimental analysis #14566

cameel · 2023-09-18T17:03:25Z

This PR removes hard-coded built-in type definitions and introduces the __builtin() construct for explicitly registering them.

It also renames the integer class to Integer to resolve the ambiguity with the integer type.

To do:

Builtin-types
Decide how to parse __builtin()

cameel · 2023-09-18T17:10:20Z

libsolidity/parsing/Parser.cpp

+			expectToken(Token::Builtin);
+			expectToken(Token::LParen);
+
+			// TMP: Do I need to guard this?
+			//if (m_scanner->currentToken() == Token::StringLiteral)
+			expression = nodeFactory.createNode<BuiltinDefinition>(
+				std::make_shared<std::string>(m_scanner->currentLiteral()),
+				// TMP: Verify that it's the right location
+				m_scanner->currentLocation()
+			);
+
+			expectToken(Token::StringLiteral);
+			expectToken(Token::RParen);


Not sure if this is the best way to parse it:

Maybe I should allow arbitrary arguments and only reject invalid ones in the analysis?

Even if not, maybe the name of the builtin should be parsed into a Literal node? Currently I only expect a string and pass it as such to the BuiltinDefinition AST node.

If it's a literal, I can make AST visit it. Not sure if that would be useful though.

cameel · 2023-09-18T17:13:34Z

test/libsolidity/syntaxTests/experimental/builtin/builtin_type_definition.sol

+type unit = __builtin("unit");
+
+type fun(T, U) = __builtin("fun");
+type pair(T, U) = __builtin("pair");


Since this is a built-in and the type variables will never be used for anything, maybe it would be better to omit them in the definition?

type fun = __builtin("fun");

I could also do this, just to keep them connected to something:

type fun(T, U) = __builtin("fun", T, U);

not sure if it makes sense though.

test/libsolidity/semanticTests/experimental/type_class.sol

cameel · 2023-09-18T17:28:33Z

libsolidity/experimental/analysis/TypeInference.cpp

-				TypeSystemHelpers helper{m_typeSystem};
-				expressionAnnotation.type =
-					helper.typeFunctionType(
-						helper.tupleType(arguments),
-						m_typeSystem.type(*constructor, arguments)
-					);


Do I still need this? I suspect I should now be doing this in visit(Identifier) when the identifier is a type name, but (at least in my simple tests) everything seems to be working just fine without it.

But maybe that's because I need more complex examples? Some expressions on types in variable declarations? Are those even implemented?

All this becoming obsolete is kind of the point in making these types defined as usual - so it should work fine (and if it doesn't yet in all cases, we'll fix that elsewhere)

But I meant this specific part of the function that I highlighted above. The assignment of a type annotation in term context is indeed handled when visiting the type definition now, but it does nothing in type context.

I'm not sure I follow exactly :-). There is (may be) some difference between the old handling as ElementaryTypeNameExpressions and the new one based on Identifiers that refer to a type declaration (which are handled in handleIdentifierByReferencedDeclaration).

But I think the new handling in Identifier should be the correct one. The old version may not even have worked for "function calls" to elementary type names in type expressions (not sure that even parsed), while the new one should.

In any case (without digging into it completely myself now), I think this is fine and if we notice that anything is missing later on, we can then fix it in the identifier visit.

cameel · 2023-09-19T13:54:37Z

Since we decided to do type classes in a separate PR, this is now ready.

ekpyron · 2023-09-19T16:08:32Z

By the way, I originally imagined to get rid of the PrimitiveType enum entirely with this and to keep TypeSystem completely oblivious to the notion of primitive types - but looking at it again now, I agree that it's better to keep this around - it doesn't exactly hurt there either and the TypeSystemHelpers would become a pain, if we got rid of TypeSystems awareness of those types entirely.
So let's keep it like this!

libsolidity/ast/AST.h

ekpyron · 2023-09-20T11:48:46Z

test/libsolidity/semanticTests/experimental/stub.sol

@@ -1,5 +1,9 @@
 pragma experimental solidity;

+type word = __builtin("word");
+type bool = __builtin("bool");


For booleans, it's funny that we could actually define them completely as user-defined type, i.e.

type bool = word;

Or later, more precisely, as an ADT

data bool = true | false;

The interesting part about booleans is rather that we have "truthiness" and "falsiness" on them, allowing them to work in conditionals.
Which is a bit of a chicken and egg problem: how to define truthiness without a boolean to indicate whether something is truthy :-)? (For the ADT representation we could just mark the true constructor as truthy and the false constructor as falsy, but this would still take some doing) So let's keep it builtin for now, but down the road it may be interesting to think about how to improve upon that.

Funnily, we wouldn't even actually need "truthiness"/"falsiness", if we defined ifs and loops in-language, which would actually work with higher-order functions and the ability to call Solidity functions from Yul if they take only types that involve "Yul-types" - but for that we'd among others need lambdas and what-not, so let's don't go there for now :-).

We might keep them as builtin permanently anyways, especially if we, in the future, introduce a boolean type in Yul (which would be one option for addressing stuff like #13750, which I moved to the optimizer ideas hackmd, but would also complicate things quite a bit, so also nothing we'd target soon)

libsolidity/experimental/analysis/TypeInference.cpp

ekpyron · 2023-09-20T15:34:55Z

I'd say it'd be nice to address #14566 (comment), but then feel free to just merge this!

- Using the same name causes syntax ambiguities. It's also not allowed for user-defined classes and types.

Defining built-ins in experimental analysis

cameel added the experimental label Sep 18, 2023

cameel requested a review from ekpyron September 18, 2023 17:03

cameel self-assigned this Sep 18, 2023

cameel commented Sep 18, 2023

View reviewed changes

test/libsolidity/semanticTests/experimental/type_class.sol Show resolved Hide resolved

cameel commented Sep 18, 2023

View reviewed changes

ekpyron mentioned this pull request Sep 19, 2023

Experimental Solidity codegen for types of different stack sizes. #14569

Open

cameel force-pushed the new-analysis-defining-builtins branch from fb10148 to aae85e8 Compare September 19, 2023 13:46

cameel marked this pull request as ready for review September 19, 2023 13:53

ekpyron reviewed Sep 20, 2023

View reviewed changes

libsolidity/ast/AST.h Outdated Show resolved Hide resolved

ekpyron reviewed Sep 20, 2023

View reviewed changes

libsolidity/experimental/analysis/TypeInference.cpp Outdated Show resolved Hide resolved

Distinct identifiers and tokens for integer type and integer class

353d50b

- Using the same name causes syntax ambiguities. It's also not allowed for user-defined classes and types.

cameel force-pushed the new-analysis-defining-builtins branch from aae85e8 to 7111950 Compare September 28, 2023 14:21

cameel requested review from r0qs and matheusaaguiar September 28, 2023 14:22

cameel mentioned this pull request Sep 28, 2023

Visiting type class bodies in experimental analysis #14578

Closed

cameel added 5 commits September 28, 2023 17:43

Syntax test for built-in types in experimental Solidity.

1072f1b

TypeSystemHelper: Add isPrimitiveType()

8304aca

Add Builtin AST node

13abd0c

Replace hard-coded types with __builtin()

0177710

Reject duplicate built-in type definitions

0819e31

cameel force-pushed the new-analysis-defining-builtins branch from 7111950 to 0819e31 Compare September 28, 2023 15:45

cameel merged commit bdd2f02 into newAnalysis Sep 28, 2023

cameel deleted the new-analysis-defining-builtins branch September 28, 2023 19:10

cameel added a commit that referenced this pull request Oct 10, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

56ad90e

Defining built-ins in experimental analysis

cameel added a commit that referenced this pull request Oct 12, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

0abd7eb

Defining built-ins in experimental analysis

cameel added a commit that referenced this pull request Oct 13, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

20db6b0

Defining built-ins in experimental analysis

cameel added a commit that referenced this pull request Oct 17, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

888aafb

Defining built-ins in experimental analysis

cameel added a commit that referenced this pull request Oct 23, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

722a98a

Defining built-ins in experimental analysis

cameel added a commit that referenced this pull request Dec 8, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

0d870e0

Defining built-ins in experimental analysis

cameel added a commit that referenced this pull request Dec 13, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

1b95edd

Defining built-ins in experimental analysis

cameel added a commit that referenced this pull request Dec 13, 2023

Merge pull request #14566 from ethereum/new-analysis-defining-builtins

4bbfd11

Defining built-ins in experimental analysis

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Defining built-ins in experimental analysis #14566

Defining built-ins in experimental analysis #14566

cameel commented Sep 18, 2023 •

edited

Loading

cameel Sep 18, 2023

cameel Sep 18, 2023

cameel Sep 18, 2023

ekpyron Sep 19, 2023

cameel Sep 19, 2023

ekpyron Sep 19, 2023

cameel commented Sep 19, 2023

ekpyron commented Sep 19, 2023

ekpyron Sep 20, 2023 •

edited

Loading

ekpyron commented Sep 20, 2023

Defining built-ins in experimental analysis #14566

Defining built-ins in experimental analysis #14566

Conversation

cameel commented Sep 18, 2023 • edited Loading

To do:

cameel Sep 18, 2023

Choose a reason for hiding this comment

cameel Sep 18, 2023

Choose a reason for hiding this comment

cameel Sep 18, 2023

Choose a reason for hiding this comment

ekpyron Sep 19, 2023

Choose a reason for hiding this comment

cameel Sep 19, 2023

Choose a reason for hiding this comment

ekpyron Sep 19, 2023

Choose a reason for hiding this comment

cameel commented Sep 19, 2023

ekpyron commented Sep 19, 2023

ekpyron Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

ekpyron commented Sep 20, 2023

cameel commented Sep 18, 2023 •

edited

Loading

ekpyron Sep 20, 2023 •

edited

Loading