compiler: temporary disallow generics usages #3041

AnnaShaleva · 2023-06-14T08:21:22Z

This PR includes the following changes:

Properly retrieve name of generic functions (fix panic described in panic: interface conversion: ast.Expr is *ast.IndexExpr, not *ast.Ident #3040).
Disallow generics usages in the following cases:
- Generic method receiver
- Generic function parameters
- Generic type declaration

AnnaShaleva · 2023-06-14T08:23:23Z

@roman-khimov, this problem is completely generics-related, I'm not sure we'd like to fix this panic right now with some temporary solution. Please, see the #3040 (comment).

codecov · 2023-06-14T08:29:49Z

Codecov Report

Merging #3041 (66a8001) into master (90f1b0f) will increase coverage by 0.08%.
Report is 10 commits behind head on master.
The diff coverage is 94.02%.

@@            Coverage Diff             @@
##           master    #3041      +/-   ##
==========================================
+ Coverage   84.78%   84.86%   +0.08%     
==========================================
  Files         329      329              
  Lines       43953    44023      +70     
==========================================
+ Hits        37265    37360      +95     
+ Misses       5189     5160      -29     
- Partials     1499     1503       +4

Files Changed	Coverage Δ
pkg/compiler/func_scope.go	`92.85% <73.33%> (-7.15%)`	⬇️
pkg/compiler/analysis.go	`96.46% <100.00%> (+0.30%)`	⬆️
pkg/compiler/codegen.go	`92.04% <100.00%> (+0.02%)`	⬆️
pkg/compiler/debug.go	`92.97% <100.00%> (ø)`

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

roman-khimov · 2023-06-14T09:34:49Z

We can just return an error if we detect any code using generics.

AnnaShaleva · 2023-06-14T09:39:11Z

getFuncNameFromDecl is used in many places (including the map keys) and returns single string. Is it worth to change all its usages as temprary solution?

roman-khimov · 2023-06-14T10:25:59Z

I don't know if there is any way to detect generic code use without going that deep. That'd be perfect, but if we don't have it then we can:

believe in this local fix to be sufficient for at least some subset of problems (maybe it really allows to compile something? maybe it'll fail in some other place?)
try leveraging c.prog.Err
try leveraging analyzeFuncAndGlobalVarUsage that looks at every function anyway and it does so before code-generating functions (btw, this seems to be easy enough to do)
at least panicing with some more meaningful message

AnnaShaleva · 2023-06-14T17:44:10Z

Close #3040.

AnnaShaleva · 2023-06-14T17:47:40Z

I've adjusted the behaviour for method receiver, now the proper error is returned to the user and it is done inside the analyzeFuncAndGlobalVarUsage before the compilation itself and #3040 is fixed by the currently presented commit. But I need some more time to carefully handle other cases of generics usages and not to break the existing code. Do we need to include these fixes to 0.102.0?

roman-khimov · 2023-06-14T19:49:13Z

If you don't feel comfortable with these changes, it's OK to postpone them for 0.102.1, no need to rush here.

AnnaShaleva · 2023-08-17T09:53:24Z

@roman-khimov, I've reworked the initial structure a bit so that it'll be easier for us to revert generics-related commits. Now all generics-related checks are separated in the checkGenerics* functions.

roman-khimov

Can we at least partially fix #3023 here? Our compiler documentation still doesn't say a word about generics.

roman-khimov · 2023-08-18T12:39:30Z

pkg/compiler/func_scope.go

+			case *ast.IndexExpr:
+				// Generic func declaration receiver: func (x *Pointer[T]) Load() *T
+				name = t.X.(*ast.IndexExpr).X.(*ast.Ident).Name + "." + name
+			}


Panic by default?

No, we won't panic here. We will retrieve the proper name and I want this code to be kept in future (unlike the rest of commits in this PR). We will analize whether the method is generic in a separate place.

Got your idea, will fix.

roman-khimov · 2023-08-18T12:39:42Z

pkg/compiler/func_scope.go

+				name = t.X.(*ast.IndexExpr).X.(*ast.Ident).Name + "." + name
+			}
+		case *ast.IndexExpr:
+			switch t.X.(type) {


Why a switch here?

Because there might be some other types, and if so, then we'll handle only the known one. Do you want to panic on something unexpected?

Yeah, there needs to be some reasonable error, otherwise we have an undefined behavior and that's not what I would like to have in the compiler.

roman-khimov · 2023-08-18T12:40:04Z

pkg/compiler/analysis.go

@@ -19,6 +19,8 @@ var (
 	ErrMissingExportedParamName = errors.New("exported method is not allowed to have unnamed parameter")
 	// ErrInvalidExportedRetCount is returned when exported contract method has invalid return values count.
 	ErrInvalidExportedRetCount = errors.New("exported method is not allowed to have more than one return value")
+	// ErrGenericsUnsuppored is returned when generics-related tokens are encountered.
+	ErrGenericsUnsuppored = errors.New("generics are currently unsupported, please, see the https://github.com/nspcc-dev/neo-go/issues/2377")


2376! Commits should reference it as well.

Fix panic described in #3040. Ref. #2376. Signed-off-by: Anna Shaleva <[email protected]>

Either non-pointer or pointer, both cases are disallowed to be generic. Need to be reverted and properly handled within the scope of #2376. Signed-off-by: Anna Shaleva <[email protected]>

Need to be reverted and properly handled within the scope of #2376. Signed-off-by: Anna Shaleva <[email protected]>

roman-khimov · 2023-08-18T13:50:38Z

docs/compiler.md

@@ -31,6 +31,8 @@ a dialect of Go rather than a complete port of the language:
 * type assertion with two return values is not supported; single return value (of the desired type)
   is supported; type assertion panics if value can't be asserted to the desired type, therefore
   it's up to the programmer whether assert can be performed successfully.
+ * type aliases including the built-in `any` alias are supported.
+ * generics are not supported, but eventually will be, ref. https://github.com/nspcc-dev/neo-go/issues/2376.


Not sure they will be supported properly, but partially maybe.

Hope, they will. At least function arguments can be supported, it doesn't seem impossible.

Part of #3023. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva force-pushed the generic-decl branch from 756b2d8 to 4ae4bcc Compare June 14, 2023 08:22

AnnaShaleva force-pushed the generic-decl branch from 4ae4bcc to 51b9f56 Compare June 14, 2023 17:42

AnnaShaleva changed the title ~~compiler: allow generic method receiver~~ compiler: disallow generic method receiver Jun 14, 2023

AnnaShaleva force-pushed the generic-decl branch from 51b9f56 to 777fcc8 Compare June 14, 2023 17:45

AnnaShaleva force-pushed the generic-decl branch from 777fcc8 to a8c2be8 Compare August 17, 2023 09:42

AnnaShaleva changed the title ~~compiler: disallow generic method receiver~~ compiler: temporary disallow generics usages Aug 17, 2023

AnnaShaleva force-pushed the generic-decl branch from a8c2be8 to febab9f Compare August 17, 2023 09:47

AnnaShaleva marked this pull request as ready for review August 17, 2023 09:52

AnnaShaleva requested a review from roman-khimov as a code owner August 17, 2023 09:52

AnnaShaleva added this to the v0.102.0 milestone Aug 18, 2023

roman-khimov requested changes Aug 18, 2023

View reviewed changes

AnnaShaleva added 4 commits August 18, 2023 16:00

compiler: properly retrieve name of generic functions

415d447

Fix panic described in #3040. Ref. #2376. Signed-off-by: Anna Shaleva <[email protected]>

compiler: disallow generic method receiver

1b1b454

Either non-pointer or pointer, both cases are disallowed to be generic. Need to be reverted and properly handled within the scope of #2376. Signed-off-by: Anna Shaleva <[email protected]>

compiler: disallow generic function parameter types

380de58

Need to be reverted and properly handled within the scope of #2376. Signed-off-by: Anna Shaleva <[email protected]>

compiler: disallow generic type decl

35c3b65

Need to be reverted and properly handled within the scope of #2376. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva force-pushed the generic-decl branch 3 times, most recently from 27edec4 to ec814da Compare August 18, 2023 13:29

AnnaShaleva requested a review from roman-khimov August 18, 2023 13:30

roman-khimov approved these changes Aug 18, 2023

View reviewed changes

docs: add note about type aliases and generics support

66a8001

Part of #3023. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva force-pushed the generic-decl branch from ec814da to 66a8001 Compare August 18, 2023 14:01

roman-khimov merged commit 227b0c5 into master Aug 18, 2023

roman-khimov deleted the generic-decl branch August 18, 2023 18:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler: temporary disallow generics usages #3041

compiler: temporary disallow generics usages #3041

AnnaShaleva commented Jun 14, 2023 •

edited

Loading

AnnaShaleva commented Jun 14, 2023 •

edited

Loading

codecov bot commented Jun 14, 2023 •

edited

Loading

roman-khimov commented Jun 14, 2023

AnnaShaleva commented Jun 14, 2023

roman-khimov commented Jun 14, 2023

AnnaShaleva commented Jun 14, 2023

AnnaShaleva commented Jun 14, 2023 •

edited

Loading

roman-khimov commented Jun 14, 2023

AnnaShaleva commented Aug 17, 2023

roman-khimov left a comment

roman-khimov Aug 18, 2023

AnnaShaleva Aug 18, 2023

AnnaShaleva Aug 18, 2023

AnnaShaleva Aug 18, 2023

roman-khimov Aug 18, 2023

AnnaShaleva Aug 18, 2023

roman-khimov Aug 18, 2023

roman-khimov Aug 18, 2023

AnnaShaleva Aug 18, 2023

roman-khimov Aug 18, 2023

AnnaShaleva Aug 18, 2023

AnnaShaleva Aug 18, 2023

compiler: temporary disallow generics usages #3041

compiler: temporary disallow generics usages #3041

Conversation

AnnaShaleva commented Jun 14, 2023 • edited Loading

AnnaShaleva commented Jun 14, 2023 • edited Loading

codecov bot commented Jun 14, 2023 • edited Loading

Codecov Report

roman-khimov commented Jun 14, 2023

AnnaShaleva commented Jun 14, 2023

roman-khimov commented Jun 14, 2023

AnnaShaleva commented Jun 14, 2023

AnnaShaleva commented Jun 14, 2023 • edited Loading

roman-khimov commented Jun 14, 2023

AnnaShaleva commented Aug 17, 2023

roman-khimov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnnaShaleva commented Jun 14, 2023 •

edited

Loading

AnnaShaleva commented Jun 14, 2023 •

edited

Loading

codecov bot commented Jun 14, 2023 •

edited

Loading

AnnaShaleva commented Jun 14, 2023 •

edited

Loading