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

Templates allowed to use ambiguous identifier #1027

Closed
gradha opened this issue Mar 22, 2014 · 0 comments · Fixed by #20631
Closed

Templates allowed to use ambiguous identifier #1027

gradha opened this issue Mar 22, 2014 · 0 comments · Fixed by #20631

Comments

@gradha
Copy link
Contributor

gradha commented Mar 22, 2014

The following set of files defines two modules with the same constant and a third one trying to echo it:

# a.nim
const version_str* = "mod a"
# b.nim
const version_str* = "mod b"
# base.nim
import a, b

echo version_str

Compilation of the base.nim file fails correctly with:

base.nim(4, 5) Error: ambiguous identifier: 'version_str' -- use a qualifier

Wrapping the ambiguous identifier in a template avoids the error:

# base.nim
import a, b

template wrap_me(stuff): stmt =
  echo "Using " & version_str

wrap_me("hey")

This executes using the symbol defined in a.nim:

$ nimrod c -r base
Using mod a
@Araq Araq added the minor label Mar 22, 2014
@Araq Araq added the Templates label Apr 22, 2018
@ringabout ringabout self-assigned this Oct 23, 2022
Araq added a commit that referenced this issue Oct 24, 2022
* test qualifiedLookUp in templates

* check later

* add testcase

* add 4errormsg

* Update tests/template/m1027a.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

* Update tests/template/m1027b.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>
ringabout added a commit that referenced this issue Feb 20, 2023
* test qualifiedLookUp in templates

* check later

* add testcase

* add 4errormsg

* Update tests/template/m1027a.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

* Update tests/template/m1027b.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>
ringabout added a commit that referenced this issue Feb 20, 2023
* Add `nkFastAsgn` into `semExpr` (#20939)

* Add nkFastAsgn into case statement

* Add test case

* fixes #1027; disallow templates to use ambiguous identifiers (#20631)

* test qualifiedLookUp in templates

* check later

* add testcase

* add 4errormsg

* Update tests/template/m1027a.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

* Update tests/template/m1027b.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

---------

Co-authored-by: Jake Leahy <jake@leahy.dev>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>
capocasa pushed a commit to capocasa/Nim that referenced this issue Mar 31, 2023
…im-lang#20631)

* test qualifiedLookUp in templates

* check later

* add testcase

* add 4errormsg

* Update tests/template/m1027a.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

* Update tests/template/m1027b.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>
bung87 pushed a commit to bung87/Nim that referenced this issue Jul 29, 2023
…im-lang#20631)

* test qualifiedLookUp in templates

* check later

* add testcase

* add 4errormsg

* Update tests/template/m1027a.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

* Update tests/template/m1027b.nim

Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>
Clyybber pushed a commit to Clyybber/Nim that referenced this issue Feb 24, 2024
## Summary

Don't use a separate storage for `SourceId` and instead store the ID
directly in `MirNode`. This is simpler, more flexible, and it prepares
for changes to the MIR's data representation.

## Details

The `SourceId` associated with a `MirNode` was previously stored in a
separate sequence, with the connection between both happening through
the index within their storing sequences (struct-of-arrays approach).

This was intended for maximum flexibility in what data representation
the source mappings use, but this potential was not realized nor is it
likely that the ID-to-struct approach is going to change.

Upcoming changes to the MIR are going to require changing/keeping meta-
data attachments on single MIR nodes when recording changesets, which
is something not easy to support ergonomically with the decoupled
storage, so the ID is stored directly in `MirNode`.

Due to the current alignment requirements of `MirNode`, this can be
done without affecting the size of the type.

**Changes:**
* change `SourceMap` to only store `SourceId` -> `PNode` mappings and
  adjust the API accordingly
* un-export `SourceMap`'s internal fields
* move `SourceId` to the `mirtrees` module
* add the `info` field to `MirNode`
* rename the `CodeFragment` type to `MirBuffer`, which better reflects
  its purpose/usage. The approach of attaching the `SourceId` in a
  deferred manner is kept, with `MirBuffer` keeping track of what nodes
  still need to be update via the `cursor` field
* export parts of the `MirBuffer` API for use in `backends` and change
  the `MirTree`-accepting `generatedCode` overload to accept a
  `MirBuffer`
* updating the attachments is still done through the `Changeset` API,
  but it now happens directly after recording a change rather than when
  applying a changeset. This is both simpler and more efficient

Documentation is updated where necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants