nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.26k stars 1.46k forks source link

[Feature Request] Add `contains`, `pop`, `dec` and more to macrocache #19494

Open EriKWDev opened 2 years ago

EriKWDev commented 2 years ago

Summary

In #11404 @jangko initially added some implementations for CacheTable.contains (and other functions) but they were removed from the PR. I couldn't find an issue mentioning them nor anything on the devel branch so I created this one. Feel free to close if already discussed elsewhere.

Description

I was in the process of converting some macros from using global {.compileTime.} variables to instead use macrocache for incremental compilation (especially wanting to use CacheTable), but found out I couldn't use string in cacheTable and others since there was no contains implementation.

Would be nice to have implementations like the ones found in Table but for CacheTable such as contains as well as other functions for CacheSeq corresponding to those for seq to ease development with macrocache.

Previously mentioned was:

I can also think of:

metagn commented 2 years ago

macrocache caches are supposedly meant to be only growable, meaning you can't have pop/dec. From what I understand you are supposed to just iterate over them once too, so other helper procs would add extra iterations that I'm not sure are supported.

EriKWDev commented 2 years ago

macrocache caches are supposedly meant to be only growable, meaning you can't have pop/dec.

I see. Would this prevent implementing ´contains´ for CacheTable as well? I don't know the details of the implementation, but supposedly that shouldn't cause an extra iteration?

Vindaar commented 2 years ago

I agree that (at least some of) these should be added.

Since I started using macrocache more, I always carry my own contains impl around (which is just a loop over the keys). The "only growable" part I understand and is fine by me.

Not sure I understand why iteration itself should be a problem (or if it is, then providing an items iterator seems broken).

metagn commented 2 years ago

I don't know the exact semantics, I was probably wrong about iterating multiple times. Currently macrocache just acts like a normal compile time global, so I don't know if anything that works now is supposed to work. (for example I do stuff like let key = table.len; table[$key] = node and I am not sure if it's supposed to work)

The original PR seemingly removed these procs because macrocache is supposed to be a low level module without any abstractions. The RFC mentions a typed wrapper without namespace strings around macrocache, so it might be better to try to do that first.

Araq commented 2 years ago

Please outline the pattern that you use contains for. I'm not against it, but we need to be careful that the data-structures remain "append-only" and what happens when there are races.

Vindaar commented 2 years ago

Please outline the pattern that you use contains for. I'm not against it, but we need to be careful that the data-structures remain "append-only" and what happens when there are races.

For me it's always a pattern as follows:

I have some macro that generates types, procedures or just some symbols. The macro cache is used to store these for the case of (these are all more or less related of course):

In all of these cases the macro logic depends on whether a key already exists or not.

I can provide explicit examples, if that helps. But the idea is always along these lines. a) avoid unnecessary computation and b) don't generate duplicate symbols.