Use === instead of == where appropriate #18853
Conversation
| @@ -528,7 +528,7 @@ end | |||
| function set_ssl_cert_locations(cert_loc) | |||
| cert_file = isfile(cert_loc) ? cert_loc : Cstring(C_NULL) | |||
| cert_dir = isdir(cert_loc) ? cert_loc : Cstring(C_NULL) | |||
| cert_file == C_NULL && cert_dir == C_NULL && return | |||
| cert_file === C_NULL && cert_dir === C_NULL && return | |||
There was a problem hiding this comment.
Good catch. Is === Cstring(C_NULL) OK?
There was a problem hiding this comment.
Maybe just remove the Cstring above.
There was a problem hiding this comment.
This is still wrong. The function also looks very strange. Is it actually setting the envs to "Cstring(0x0000000000000000)"?
julia> ENV["AAA"] = Cstring(C_NULL)
Cstring(0x0000000000000000)
julia> ENV["AAA"]
"Cstring(0x0000000000000000)"The setting of the ENV should probably just be folded with the isdir/isfile check (can they possibly both pass?) and clear the corresponding variable if necessary.
There was a problem hiding this comment.
Funny. Cc: @simonbyrne who recently worked on libgit2.
| @@ -786,8 +786,8 @@ mktempdir() do dir | |||
| rethrow(err) | |||
| finally | |||
| rm(logfile) | |||
| sshp !== nothing && kill(sshp) | |||
| agentp !== nothing && kill(agentp) | |||
| sshp !=== nothing && kill(sshp) | |||
There was a problem hiding this comment.
This is disabled but is also wrong.
| @@ -385,12 +385,12 @@ function exprresolve(ex::Expr) | |||
| can_eval, result = exprresolve_arith(ex) | |||
| if can_eval | |||
| return result | |||
| elseif ex.head == :call && (ex.args[1] == :+ || ex.args[1] == :-) && length(ex.args) == 3 && ex.args[3] == 0 | |||
| elseif ex.head === :call && (ex.args[1] === :+ || ex.args[1] === :-) && length(ex.args) == 3 && ex.args[3] === 0 | |||
There was a problem hiding this comment.
this could be a slight behavior change. wonder where this happens, there might be types that aren't closed under addition where this is invalid
There was a problem hiding this comment.
Sorry, I'm not following (I don't know what that code does). Are you talking about the === 0 part?
There was a problem hiding this comment.
It used to accept anything that's == 0 as ex.args[3].
There was a problem hiding this comment.
OK, that I understand. I'll change it back unless we're 100% sure that's not an issue.
There was a problem hiding this comment.
@timholy Do you think this is OK? From the comment on line 384, it seems this should only handle literal 0, so should be fine.
There was a problem hiding this comment.
This is a behavior change, which is not the purpose of this commit, so I think it should be changed back.
| @@ -385,12 +385,12 @@ function exprresolve(ex::Expr) | |||
| can_eval, result = exprresolve_arith(ex) | |||
| if can_eval | |||
| return result | |||
| elseif ex.head == :call && (ex.args[1] == :+ || ex.args[1] == :-) && length(ex.args) == 3 && ex.args[3] == 0 | |||
| elseif ex.head === :call && (ex.args[1] === :+ || ex.args[1] === :-) && length(ex.args) == 3 && ex.args[3] === 0 | |||
There was a problem hiding this comment.
This is a behavior change, which is not the purpose of this commit, so I think it should be changed back.
|
I had completely forgotten about this PR. I've rebased and reverted the change in behavior, so should be good to merge now. |
| @@ -528,7 +528,7 @@ end | |||
| function set_ssl_cert_locations(cert_loc) | |||
| cert_file = isfile(cert_loc) ? cert_loc : Cstring(C_NULL) | |||
| cert_dir = isdir(cert_loc) ? cert_loc : Cstring(C_NULL) | |||
| cert_file == C_NULL && cert_dir == C_NULL && return | |||
| cert_file === C_NULL && cert_dir === C_NULL && return | |||
There was a problem hiding this comment.
This is still wrong. The function also looks very strange. Is it actually setting the envs to "Cstring(0x0000000000000000)"?
julia> ENV["AAA"] = Cstring(C_NULL)
Cstring(0x0000000000000000)
julia> ENV["AAA"]
"Cstring(0x0000000000000000)"The setting of the ENV should probably just be folded with the isdir/isfile check (can they possibly both pass?) and clear the corresponding variable if necessary.
Continuation of #16764.