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 we merge two assert statements? #8856

Closed
IainNZ opened this issue Oct 30, 2014 · 12 comments
Closed

Can we merge two assert statements? #8856

IainNZ opened this issue Oct 30, 2014 · 12 comments
Labels
deprecation This change introduces or involves a deprecation good first issue Indicates a good issue for first-time contributors to Julia help wanted Indicates that a maintainer wants help on an issue or pull request
Milestone

Comments

@IainNZ
Copy link
Member

IainNZ commented Oct 30, 2014

So we have @assert

julia> @assert 1==2 "It doesnt"
ERROR: assertion failed: It doesnt
 in error at error.jl:21

and assert, which can't take a second argument,

julia> assert(1==2)
ERROR: assertion failed

Is there any need to keep both?

@JeffBezanson
Copy link
Member

Doesn't seem so. The assert() form probably exists to match C, but that's not such a big deal.

@elextr
Copy link

elextr commented Oct 30, 2014

The manual notes the macro is the preferred, but doesn't say why.

@IainNZ IainNZ added help wanted Indicates that a maintainer wants help on an issue or pull request good first issue Indicates a good issue for first-time contributors to Julia labels May 25, 2015
@IainNZ
Copy link
Member Author

IainNZ commented May 25, 2015

Could be good to have PR that deprecates the assert form, and just leaves the macro version. Then the macro version could annotate the expression so that it could be, e.g. not run in a "high performance" mode

@mzaffalon
Copy link
Contributor

Beginner's questions:

  • does this imply going through the code and replacing each assert with @assert and adding it to deprecated.jl? I assume not: I quickly did so and it was not sufficient.
  • what is the annotation you talk about?
    I can give this issue a try if someone can give some guidance.

@tkelman
Copy link
Contributor

tkelman commented Aug 6, 2015

I quickly did so and it was not sufficient.

In what way was it not sufficient? What happened vs what did you expect?

I don't think the annotation exists as such yet, but there has been talk and one or two other issues on the idea of using compiler options or flags to set different debugging levels for whether assertion statements get skipped, etc. If people want to make asserts fancier in the future, would be easier if it only had to be done to the macro form instead of both the macro and the function.

@IainNZ
Copy link
Member Author

IainNZ commented Aug 6, 2015

@michele-zaffalon

My vision was, for the deprecation, to

  • deprecate assert()
  • make @assert call an internal _assert() (if thats even needed)
  • replace usages in Base of assert with @assert

For annotation, I know that @simd does something with $(Expr(:simdloop)), and I assumed there was something similar for @inbounds, so such a thing would seemingly work for @assert. Alternatively, @assert generates code that tests if a constant global if true or false, and then the compiler would remove the dead code if its false.

@mzaffalon
Copy link
Contributor

@tkelman Here is what I did:

  • added assert to deprecated.jl
  • removed assert from exports.jl
  • replaced assert with @assert in inference.jl
  • built julia: all goes fine

When I do make testall, I get the following:

michele@gulliver:~/projects/julia/julia$ make testall
cp /home/michele/projects/julia/julia/usr/lib/julia/sys.so local.so && /home/michele/projects/julia/julia/usr/bin/julia -J local.so -e 'true' && rm local.so
WARNING: assert(x) is deprecated, use @assert(x) instead.WARNING: error initializing module Base:
OutOfMemoryError()
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
jl_uv_writecb() ERROR: bad file descriptor EBADF
rec_backtrace at /home/michele/projects/julia/julia/usr/bin/../lib/libjulia.so (unknown line)
jl_throw at /home/michele/projects/julia/julia/usr/bin/../lib/libjulia.so (unknown line)
unknown function (ip: 0x7f08ba3e099c)
unknown function (ip: 0x7f08ba3e09a9)
uv_writecb_task at stream.jl:924
jlcapi_uv_writecb_task_20924 at  (unknown line)
uv__stream_destroy at /home/michele/projects/julia/julia/usr/bin/../lib/libjulia.so (unknown line)
unknown function (ip: (nil))
Makefile:514: recipe for target 'testall' failed
make: *** [testall] Terminated
Terminated

@mzaffalon
Copy link
Contributor

@IainNZ There doesn't seem to be a need for an _assert function, but you people are more experienced and knowledgeable, so you say what I have to do.

For the second part, I am not good enough to do it myself.

@JeffBezanson JeffBezanson added triage This should be discussed on a triage call deprecation This change introduces or involves a deprecation labels Feb 16, 2018
@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Feb 22, 2018
@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Feb 22, 2018
@JeffBezanson
Copy link
Member

Triage thinks we should deprecate assert (the function form). It's more consistent with e.g. the new logging macros.

@StefanKarpinski
Copy link
Member

Triage accepts: deprecate assert to use the @assert macro.

@twadleigh
Copy link
Contributor

I'll take a stab at this today.

twadleigh pushed a commit to twadleigh/julia that referenced this issue Feb 24, 2018
twadleigh pushed a commit to twadleigh/julia that referenced this issue Feb 24, 2018
twadleigh pushed a commit to twadleigh/julia that referenced this issue Feb 24, 2018
twadleigh pushed a commit to twadleigh/julia that referenced this issue Feb 24, 2018
twadleigh pushed a commit to twadleigh/julia that referenced this issue Feb 24, 2018
twadleigh pushed a commit to twadleigh/julia that referenced this issue Feb 24, 2018
twadleigh pushed a commit to twadleigh/julia that referenced this issue Feb 24, 2018
twadleigh added a commit to twadleigh/julia that referenced this issue Feb 25, 2018
twadleigh added a commit to twadleigh/julia that referenced this issue Feb 25, 2018
twadleigh added a commit to twadleigh/julia that referenced this issue Feb 25, 2018
@StefanKarpinski
Copy link
Member

Closed by #26194.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation good first issue Indicates a good issue for first-time contributors to Julia help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

No branches or pull requests

7 participants