-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix #12150 C++ free functions are now supported using new importcpp:"!$1"
syntax
#15653
fix #12150 C++ free functions are now supported using new importcpp:"!$1"
syntax
#15653
Conversation
importcpp:"!$1"
syntax
3ac73a6
to
2f63f57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be handled in ccgcalls.genPatternCall
instead.
this wouldn't help with #12150, try it; one problem was that |
Why doesn't |
because it would use c mangling. If you have a C++ free function, (eg: |
Please explain this in more detail. |
@Araq this line should explain everything:
in the example of #12150, after PR, I distinguish C++ free functions (which need a declaration) vs methods ( |
Ok, but I meant to ask:
Why can't it be done in genPatternCall? Seems the most obvious place to put it. |
I don't see how that would work, at very least I don't see how it would be simpler than the way I wrote it, if you have a concrete idea in mind please provide more details / edit this PR. I need to tell whether an importcpp symbol is sfInfixCall or not early on (inside let isFreeFunction = extname.startsWith "!"
if isFreeFunction:
extname = extname[1..^1]
else:
incl(s.flags, sfInfixCall) and interface of |
e60c014
to
df242a0
Compare
Hi @Araq anything else needed here? I rebased to fix conflicts |
df242a0
to
7759b03
Compare
rebased again to fix bitrot conflicts... I'd really like to have this merged |
ping @xflywind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
7759b03
to
18dc326
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL
@Araq any other comments or can we merge this? |
Sorry for being late but if we really need this, we should have a dedicated |
maybe we need eg: proc freeFn(a: cint) {.pattern: "$call".} # whole call; honors backend mangling (c/c++/js)
proc freeFn2(a, b: cint) {.pattern: "std::bar(@)".} # calls std::bar(a, b)
proc `+`*(a, b: int128): int128 {.pattern: "$1 + $2".} # calls a + b
proc `+!`*(a, b: int128): int128 {.pattern: "$2 + $1".} # calls b + a
proc freeFn3(a, b: cint) {.pattern: "$0(1, @)".} # calls freeFn3(1, a, b) |
Either |
fix #12150
example