deprecate macros.expectLen, expectKind, expectMinLen in favor of something generic or doAssert #185
Comments
timotheecour
changed the title
|
Sorry, I use the expectX family of procs regularly. Every macro that I write uses them. |
see example:
nim c --hint:source:on main.nim import macros
macro fun(n1, n2) =
result = newStmtList()
result.add quote do:
[1,2]
expectLen result, 3
macro baz(): untyped = newCall("fun", newLit 1, newLit 2)
baz()with expectLen:
the doAssert error msg isn't missing any relevant information (at least none that expectLen shows) |
|
bad example. here is a list of examples where the expect procs are used in the project I have currently in my working directory. |
|
Since someone wanted opinions... IMHO, a generic ast-aware solution is a good idea, but you have to try harder to satisfy existing code. |
It shows the actual len now.
Your example is specially crafted to fail this way..
Its about making often used stuff convinient. And providing coherent and nice error messages.
This is not an issue. Its not like expectKind modifies kind..
It should be always on. It would be really bad if my macro fails silently because I compiled with -d:danger.
Fine, you can easily "fix" that though.
In general this RFC feels a bit like: Use tires instead of cars :p |
|
As something of an alternative, I wrote some utility macros for use when writing macros ("...we must go deeper..."), which allow me to write code like this: if proto =~ Infix(Ident("->"), [], Ident(_)):
rett = proto[2].handleJavaType
proto = proto[1]
# ...class & method name:
if proto !~ Call(_):
error "jproto expects a method declaration as an argument", proto
if proto !~ Call(DotExpr(_), _):
error "jproto expects dot-separated class & method name in the argument", proto
if proto[0] !~ DotExpr(Ident(_), Ident(_)) and
proto[0] !~ DotExpr(Ident(_), AccQuoted(_)):
error "jproto expects exactly 2 dot-separated names in the argument", proto[0]where:
They would definitively need some polishing etc. if they were to be e.g. considered for merging into stdlib, but even if not, I think they may work as an interesting example for discussion; I personally find them nice from readability point of view. |
or: use 1 car that's good enough to drive to multiple destinations instead of having to change car for every route. In all seriousness, if doAssert isn't good enough (still not convinced on that but assuming), can't we at least have a single expectLen(n, 2) => expect n.len == 2
expectMinLen(n, 2) => expect n.len >= 2and expect would then support everything that's supported by the expect would also take an optional msg param for when extra runtime info is needed, but that's a separate issue that can also be supported in each expectX proc |
|
@akavel looks like you have invented ast pattern matching as well. |
|
While I see the value in improving the expectX procs/interface I think our attention is better spent on gc:arc, "on and dup" (chaining and outplace) (got an RFC in development), var/lets as alias, incremental recompilation, cmake integration .... well you get the idea :-) |
|
@timotheecour Please close this RFC, the improvement would be too minor and it eats up attention. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I don't think the proliferation of
expectX(eg: expectLen, expectKind, expectMinLen, expectIdent (nim-lang/Nim#12778)) makes sense in a language like nim that has macros and can do this generically.expectLen(n, 2)doesn't even show the valuen.lenin the error msg in case of failure (granted, this is easy to fix)expectKind(n, nnkSym)hides the fact that n.kind is accessed, eg where searching for accessors of a fieldkindproposal 1
noted, expectKind (unlike expectLen) does show the input kind, but we can do:
proposal 2
if assert/doAssert is not enough (say because it doesn't automatically show input value unless you specify it eg via
doAssert(n.kind == nnkSym, $n.kind)):we can automatically show the relevant input arg deconstructed from AST (eg n.kind == nnkSym) in same way as done in unittest.expect;
either we can reuse unittest.expect, or we can introduce a new "simplified" version of unittest.expect that just also shows the values of arguments in simple cases (eg infix), but doesn't add dependency on std/unittest, eg:
doExpect(n.kind == nnkSym)The text was updated successfully, but these errors were encountered: