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

some issues with hasCustomPragma/recordFields #12

Open
jangko opened this issue Nov 7, 2019 · 4 comments
Open

some issues with hasCustomPragma/recordFields #12

jangko opened this issue Nov 7, 2019 · 4 comments

Comments

@jangko
Copy link
Contributor

jangko commented Nov 7, 2019

  1. getType(T)[1].getImpl will not work on ref type including deref version like type(default(someref)[]).

possible work around. after getType, use new owner API then symKind and typeKind to check if it has parent type or something else.

nim-confutils, nim-serialization, and nim-json-serialization suffer from this problem too.

  1. nnkPragma and nnkPosfix in recordFields
          if field.name.kind == nnkPragmaExpr:
            field.pragmas = field.name[1]
            field.name = field.name[0]

          if field.name.kind == nnkPostfix:
            field.isPublic = true
            field.name = field.name[1]

this is clearly wrong, nnkPostFix will appear inside nnkPragmaExpr in the AST. need fix

  1. using nnkRecWhen in recordFields can leads to unexpected result. should use semchecked AST of getType instead of raw AST from getImpl.

nnkRecWhen will not appear at final type, only branch evaluated to true will appear at final type. if we use nnkRecWhen, both false branch and true branch will have conflict if they contains fields with same name. false branch existence also posses it's own problem if we count the field number or enumerate the fields.

work around: not known, getType and friends return an AST without pragmas. need to patch the getType family.

related: nim-lang/RFCs#176

@zah
Copy link
Contributor

zah commented Nov 8, 2019

this is clearly wrong, nnkPostfix will appear inside nnkPragmaExpr in the AST. need fix

But that's what the code is doing now. It first strips the "outer" layer, which is the nnkPragmaExpr, then moves on to strip the inner one. I believe this is code path is already tested in confutils.

  1. The nnkRecWhen problem is a tough one. What I really need is the Nim compiler to offer an option to handle this for me and to return the fields in the record after when was applied.

@jangko
Copy link
Contributor Author

jangko commented Nov 8, 2019

But that's what the code is doing now. It first strips the "outer" layer, which is the nnkPragmaExpr, then moves on to strip the inner one. I believe this is code path is already tested in confutils.

sorry, you're right.I must be confused with something else

@jlokier
Copy link

jlokier commented Dec 10, 2021

@zah:

The nnkRecWhen problem is a tough one. What I really need is the Nim compiler to offer an option to handle this for me and to return the fields in the record after when was applied.

I have just encountered this when issue after adding a confutils option to nimbus-eth1 that should only be present in some build configurations. We didn't have this problem in the past, but now we've switched to confutils we do.

Is there a way to make when work nowadays? The conditional is a very simple constant, when defined(evmc_enabled), and the options list it fits into is long (and there will be more conditionally-compiled options I expect).

@jangko
Copy link
Contributor Author

jangko commented Dec 13, 2021

@jlokier: now nim-confutils already support ignore pragma in status-im/nim-confutils#37, the closest workaround to achieve conditional fields is to use pragma-pragma

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants