Fixes handling of nkBracketExpr nodes in opcGetImpl.

[dom96]

This fixes a problem that is similar to #8484. I attempted to
reproduce it via:

import macros

template custom {.pragma.}

type
  Vec[T] = object
    x, y: T
  Entry = object
    c: Vec[float]
    a {.custom.}: int
    b: float

proc foo(o: object) =
  for name, value in o.fieldPairs:
    when value.hasCustomPragma(custom):
      echo(name, " has {.custom.}")

var x: Entry
foo(x)

But was not able to do so, possibly just a case of my code having
nesting of templates/generics. But this simple patch fixes the
compilation of my code.

[Vindaar]

If I understand you correctly, the code you posted as an example does not show the error?

If so, that makes it sounds like it might be either related to:

• Custom pragma ignored on field of variant obj if in multiple branches #11415, which I fixed in 28bb565
• or to what I encountered here: 5da931f, since the fix is the same idea, except in macros.

[krux02]

I can't reproduce a problem in your code. What is your output? What output do you expect?

[dom96]

I got "node is not a symbol" errors in my code (raised just below the line I modified), I checked the node it was failing on and it's nkBracketExpr.

And yes, I tried to reproduce it with a smaller example but couldn't (the code I share above is as far as I've gotten to). I've got some code which I cannot share which runs into this. Is there any reason why we cannot allow this change?

[dom96]

@Vindaar

or to what I encountered here: 5da931f, since the fix is the same idea, except in macros.

huh, indeed, this looks like it would fix my problem. But I cannot see it in devel, I cannot figure out what happened to your commit: https://github.com/nim-lang/Nim/blame/devel/lib/core/macros.nim#L1475

The reason I did it in the compiler is because of how #8484 was fixed (and I wanted to quickly create a PR that fixes it). But actually after looking at it more closely I do think this belongs in the macros module. I will change it to recreate your commit.

[Vindaar]

Ah, sorry for the confusion. Both of the commits I referenced above are part of the PR #11416. I didn't realize the commits show up seemingly by themselves.

[krux02]

As I said, I can't reproduce the problem, this is my output:

-*- mode: compilation; default-directory: "/tmp/" -*-
Compilation started at Wed Aug  7 23:11:49

nim c -r scratch.nim
Hint: used config file '~/proj/nim/Nim/config/nim.cfg' [Conf]
Hint: used config file '~/.config/nim/nim.cfg' [Conf]
Hint: used config file '~/proj/nim/Nim/config/config.nims' [Conf]
Hint: system [Processing]
Hint: widestrs [Processing]
Hint: io [Processing]
Hint: scratch [Processing]
Hint: macros [Processing]
CC: stdlib_system.nim
CC: scratch.nim
Hint:  [Link]
Hint: operation successful (23123 lines compiled; 0.361 sec total; 26.004MiB peakmem; Debug Build) [SuccessX]
Hint: /tmp/scratch  [Exec]
a has {.custom.}

Compilation finished at Wed Aug  7 23:11:50

Apart from that, bug fixes should have a test case attached to them.

[Vindaar]

@krux02: You misunderstand. The code Dom provided in the OP was his attempt at a simple example to reproduce the problem, which however fails to do so (while being similar in idea to his original code, I imagine).

And as I mentioned in my commit 5da931f, I wasn't able to write a test case for it either. In that case the problem was an already existing test, namely this one: https://github.com/nim-lang/Nim/blob/devel/tests/stdlib/tjsonmacro.nim#L7-L55

On devel that code obviously passes the tests, but it started throwing the same error Dom mentions "node is not a symbol", after this commit: 28bb565.

[dom96]

Yes, what @Vindaar said. Also, test failures look flaky (the failure is frankly ridiculous: https://travis-ci.org/nim-lang/Nim/jobs/568557269#L6).

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.