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

Can't attach a pragma to a cased field. #14511

Closed
kayabaNerve opened this issue May 30, 2020 · 9 comments · Fixed by #18732
Closed

Can't attach a pragma to a cased field. #14511

kayabaNerve opened this issue May 30, 2020 · 9 comments · Fixed by #18732

Comments

@kayabaNerve
Copy link
Collaborator

kayabaNerve commented May 30, 2020

Attaching a pragma to a field used in a case within an object has no effect on the AST.

Example

import macros

template myPragma() {.pragma.}

type
  XType = enum
    XInt,
    XString
  X = object
    case y {.myPragma.}: XType
      of XInt:
        a: int
      else:
        b: string
  
var x: X = X(y: XInt, a: 5)
echo x.y.hasCustomPragma(myPragma)
echo x
echo X(y: XString, b: "abc")

Current Output

nim/lib/core/macros.nim(1546, 25) Error: index 1 not in 0 .. 0

Expected Output

true
(y: XInt, a: 5)
(y: XString, b: "abc")

Additional Information

I did also try X.y.hasCustomPragma(myPragma); that compiles, yet returns false.

$ nim -v
Nim Compiler Version 0.1.2
@ghost
Copy link

ghost commented May 30, 2020

It's a known bug #11526

@kayabaNerve
Copy link
Collaborator Author

While that seems to cover hasCustomPragma being problematic, it doesn't seem to address this specific issue.

@ghost
Copy link

ghost commented May 30, 2020

@ghost
Copy link

ghost commented May 30, 2020

The main trick there is that you have to do "yourSym.owner.getImpl()[2]" where yourSym is a symbol of a field of the object you want to get pragma of.

@kayabaNerve
Copy link
Collaborator Author

Got it; I read through the discussion, not the source. Thanks for pointing that out; should I close this issue?

@kayabaNerve
Copy link
Collaborator Author

Thanks for the workaround.

@ghost
Copy link

ghost commented May 30, 2020

@kayabaNerve not sure, probably you should keep it open unless that PR is merged :)

@kayabaNerve
Copy link
Collaborator Author

Sure. Thanks again for linking the relevant PR and a workaround.

@kayabaNerve
Copy link
Collaborator Author

Sorry, was browsing this issue on my phone and clicked the wrong button.

Dankr4d added a commit to Dankr4d/Nim that referenced this issue Aug 22, 2021
Signed-off-by: Dankr4d <dude569@freenet.de>
Dankr4d added a commit to Dankr4d/Nim that referenced this issue Aug 22, 2021
Signed-off-by: Dankr4d <dude569@freenet.de>
Dankr4d added a commit to Dankr4d/Nim that referenced this issue Aug 22, 2021
Signed-off-by: Dankr4d <dude569@freenet.de>
Dankr4d added a commit to Dankr4d/Nim that referenced this issue Aug 23, 2021
Signed-off-by: Dankr4d <dude569@freenet.de>
alaviss pushed a commit that referenced this issue Aug 25, 2021
* fixes #14511 [backport:1.4]

Signed-off-by: Dankr4d <dude569@freenet.de>

* Replaced fix with code from alaviss, for better readability, with small
changes.

Signed-off-by: Dankr4d <dude569@freenet.de>

* - Specified output in test.

Signed-off-by: Dankr4d <dude569@freenet.de>

* Replaced case in nnkRecCase with a simpler version, which just adds the
last son.

Signed-off-by: Dankr4d <dude569@freenet.de>

* Update tests/macros/t14511.nim

* Update tests/macros/t14511.nim

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
narimiran pushed a commit that referenced this issue Sep 2, 2021
* fixes #14511 [backport:1.4]

Signed-off-by: Dankr4d <dude569@freenet.de>

* Replaced fix with code from alaviss, for better readability, with small
changes.

Signed-off-by: Dankr4d <dude569@freenet.de>

* - Specified output in test.

Signed-off-by: Dankr4d <dude569@freenet.de>

* Replaced case in nnkRecCase with a simpler version, which just adds the
last son.

Signed-off-by: Dankr4d <dude569@freenet.de>

* Update tests/macros/t14511.nim

* Update tests/macros/t14511.nim

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit c70e404)
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
* fixes nim-lang#14511 [backport:1.4]

Signed-off-by: Dankr4d <dude569@freenet.de>

* Replaced fix with code from alaviss, for better readability, with small
changes.

Signed-off-by: Dankr4d <dude569@freenet.de>

* - Specified output in test.

Signed-off-by: Dankr4d <dude569@freenet.de>

* Replaced case in nnkRecCase with a simpler version, which just adds the
last son.

Signed-off-by: Dankr4d <dude569@freenet.de>

* Update tests/macros/t14511.nim

* Update tests/macros/t14511.nim

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
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

Successfully merging a pull request may close this issue.

1 participant