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 #19546 Missing compiler error for array in type definition (also … #20412

Closed
wants to merge 10 commits into from

Conversation

bung87
Copy link
Collaborator

@bung87 bung87 commented Sep 23, 2022

…apply to seq, set)
fix #19546
fix #13992
fix #19164
fix #6982

related: #15993

@beef331
Copy link
Collaborator

beef331 commented Sep 24, 2022

As you're on the quest of fixing these issues, I will note that openarray, sink, and lent also compile without complaint.

@bung87
Copy link
Collaborator Author

bung87 commented Sep 24, 2022

As you're on the quest of fixing these issues, I will note that openarray, sink, and lent also compile without complaint.

could you provide example? that's may not related , here are fix for type declaration and variable declaration, types your provide are in proc arguments.

@beef331
Copy link
Collaborator

beef331 commented Sep 24, 2022

type A = object
  a: openarray
  b: sink
  c: lent

@Varriount Varriount requested a review from Araq September 25, 2022 18:53
@Varriount Varriount added the Requires Araq To Merge PR should only be merged by Araq label Sep 25, 2022
compiler/semexprs.nim Outdated Show resolved Hide resolved
@bung87
Copy link
Collaborator Author

bung87 commented Sep 28, 2022

CI fails cause by https://github.com/SciNim/numericalnim/blob/614cb304d74ac32869ff74e0d013681ecf4ff5c8/src/numericalnim/integrate.nim#L420

suprised this syntax is used in real project.

const gaussWeights: array[1..20, (seq, seq)] = [
  (@[0.0],
  @[2.0]),

@bung87
Copy link
Collaborator Author

bung87 commented Sep 28, 2022

@beef331 told me proc doThing(a: seq, b: set, c: array) is valid , but I didnot consider that. no idea for now.

@bung87 bung87 closed this Sep 29, 2022
@bung87 bung87 reopened this Sep 29, 2022
@metagn
Copy link
Collaborator

metagn commented Sep 29, 2022

Maybe a simpler solution is just to put tyNone in tyTypeClasses/tyMetaTypes?

Or, if we want to error just for object fields, change t.isMetaType in checkForMetaFields to (t.isMetaType or t.kind == tyNone).

@bung87
Copy link
Collaborator Author

bung87 commented Sep 30, 2022

Maybe a simpler solution is just to put tyNone in tyTypeClasses/tyMetaTypes?

Or, if we want to error just for object fields, change t.isMetaType in checkForMetaFields to (t.isMetaType or t.kind == tyNone).

good idea, let's hear morevoice.

@dlesnoff
Copy link
Contributor

dlesnoff commented Oct 1, 2022

Would it be possible, in case of a type definition, to add the field's name in the error message ?

@bung87
Copy link
Collaborator Author

bung87 commented Oct 2, 2022

Would it be possible, in case of a type definition, to add the field's name in the error message ?

it's possible. let's see @Araq 's opinion.

@bung87 bung87 force-pushed the fix19546 branch 2 times, most recently from 6dee7a4 to 737d146 Compare October 2, 2022 07:55
@dlesnoff
Copy link
Contributor

dlesnoff commented Oct 2, 2022

Tested your code on some examples.

type
  myObject = object
    tab: seq

Now fails gracefully with a seq is not a concrete type. Thanks !

type
  myObject[T] = object
    tab: seq

Why does this not fail ?!

type
  someGeneric[T] = object
    field: seq
type
  myObject = object
    tab: someGeneric

Fails with someGeneric is not a concrete type. Should IMO fail at someGeneric type definition, just like previous example.

@dlesnoff
Copy link
Contributor

dlesnoff commented Oct 2, 2022

Here is a currently problematic error description:

type
  someGeneric[U, V] = object
    a: seq[U]
    b: seq
  myObject = object
    tab: someGeneric[int, string]

got: ~/Documents/dev/typeGenericsIssue.nim(6, 5) Error: 'someGeneric' is not a concrete type
The error is not in tab's field but in field b of someGeneric type definition

@bung87
Copy link
Collaborator Author

bung87 commented Oct 2, 2022

Here is a currently problematic error description:

type
  someGeneric[U, V] = object
    a: seq[U]
    b: seq
  myObject = object
    tab: someGeneric[int, string]

got: ~/Documents/dev/typeGenericsIssue.nim(6, 5) Error: 'someGeneric' is not a concrete type The error is not in tab's field but in field b of someGeneric type definition

I just re checked , generic can't check without instantiating, they will not turns to a node for check. I added a good case , the first generic type will not checked, but it can be used in second type. second one is plain object, it do check its definition.

@bung87 bung87 requested a review from Araq October 6, 2022 17:41
@bung87
Copy link
Collaborator Author

bung87 commented Oct 22, 2022

@Araq tyNone is added in semMagic.

@bung87
Copy link
Collaborator Author

bung87 commented Nov 29, 2022

@Araq what else I can do for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Requires Araq To Merge PR should only be merged by Araq
Projects
None yet
6 participants