https://issues.dlang.org/show_bug.cgi?id=19570

Nicholas Wilson <iamthewilsonator@hotmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |iamthewilsonator@hotmail.co
                   |                            |m

--- Comment #1 from Nicholas Wilson <iamthewilsonator@hotmail.com> ---
https://github.com/dlang/dmd/pull/9235

--

https://issues.dlang.org/show_bug.cgi?id=19570

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com

--- Comment #2 from Walter Bright <bugzilla@digitalmars.com> ---
Why is this marked as critical?

Besides, the reason it is emitted to the object file is in the case someone in another module takes the address of it.

Also, the linker is supposed to elide unreferenced COMDATs, so it should go away.

This needs a better rationale for why it is a bug, let alone critical.

--

https://issues.dlang.org/show_bug.cgi?id=19570

Manu <turkeyman@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal

--- Comment #3 from Manu <turkeyman@gmail.com> ---
I'm not sure why it's critical, I don't recall doing that... Perhaps it was an accident. I created another bug critical yesterday, but that was bad codegen, failing to do the ABI proper.

> Besides, the reason it is emitted to the object file is in the case someone in another module takes the address of it.

>From another module being emit to the same object file, or another module
compiled into a separate object file?
I would expect taking the address would cause the code to be emit locally to
the module that's taking the address, rather than externing to a copy in the
module it's declared.

> Also, the linker is supposed to elide unreferenced COMDATs, so it should go away.

It doesn't. That's motivating this issue. But also I'm upset that it was there in the first place. That's distinctly *not* honouring my "inline" request.

> This needs a better rationale for why it is a bug, let alone critical.

I mean, I don't feel like I should be presenting an argument here... an explicit inline feature that doesn't do what's written on the tin seems like a bug.

I think the rationale needs to support why it wouldn't behave the way you'd expect.

Suggesting that we have to emit the function because a 3rd party should be able
to link to it is weird, because that's totally self-defeating.
Why is it inline then? I expect that if I've explicitly marked it inline, then
I can not extern to it, that would be the whole point.

Code that calls an inline function should either inline it as preferred, or if it decides not to for some reason, emit the code locally into the referencing module (and perhaps even don't emit a symbol name for the not-inline code).

--

https://issues.dlang.org/show_bug.cgi?id=19570

--- Comment #4 from Manu <turkeyman@gmail.com> ---
I recall having a big discussion about this when you added it... and I felt like we agreed on this and the semantics were very clear. You were even in favour at the time of requiring a compile error if inline failed, but I think I resisted on a hard error, because other compilers weren't necessarily able to respect that request.

--

https://issues.dlang.org/show_bug.cgi?id=19570

Manu <turkeyman@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |critical

--- Comment #5 from Manu <turkeyman@gmail.com> ---
It is critical, inline literally does nothing at all, as shows in the comments
section of the PR threads:
https://github.com/dlang/dmd/pull/9235

Absolutely nothing about inline works, and I'm blocked on it.

--

https://issues.dlang.org/show_bug.cgi?id=19570

Witold Baryluk <witold.baryluk+d@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |witold.baryluk+d@gmail.com

--- Comment #6 from Witold Baryluk <witold.baryluk+d@gmail.com> ---
I think you are confusing function being marked inline, and about non-emiting object code of it or marking it weak. These are independent.

Compiler is free to ignore all `pragma(inline, ...)` statements, at any optimization / flags settings.

What you want are `private final` symbol, or way to mark it weak. That would be a better signal to compiler and linker.

`pragma(inline, ...)` is used for other things.

It even says so in the spec:

https://dlang.org/spec/pragma.html#inline

"""
Implementation defined:

2. Whether a particular function can be inlined or not is implementation
defined.
3. What happens for pragma(inline, true) if the function cannot be inlined. An
error message is typical.
"""

DMD default to emitting an error and aborting compilation when -inline flag is used. IMHO that is meh solution, I would like warning and continue with compilation.

--

https://issues.dlang.org/show_bug.cgi?id=19570

--- Comment #7 from Manu <turkeyman@gmail.com> ---
There's a lot of noise at the start of this chain.
Ignore everything else I've said, let me simplify:

`inline` should:

 - emit a copy of the function to each CU it's called from
 - not emit the function to the CU it's declared (unless it's called, as above)
 - mark the function with 'internal' linkage (what C++ calls 'static') so it's
not in the object's exported symbol table.
 - *optionally* apply a strong hint to encourage true inlining - this is nice,
but it's secondary to the requirements above


Practical features:

 - symbol is not externally linkable
 - symbols referenced by an inline function that's never called must NOT cause
a link error under any circumstances

--

https://issues.dlang.org/show_bug.cgi?id=19570

--- Comment #8 from Walter Bright <bugzilla@digitalmars.com> ---
(In reply to Manu from comment #7)
>  - emit a copy of the function to each CU it's called from

But then it's going to be doing code gen for each of them over and over.

>  - not emit the function to the CU it's declared (unless it's called, as
> above)
>  - mark the function with 'internal' linkage (what C++ calls 'static') so
> it's not in the object's exported symbol table.

So what if it's in the symbol table?

> - symbol is not externally linkable

Why not?

> - symbols referenced by an inline function that's never called must NOT cause a link error under any circumstances

This is a linker problem - the linker is supposed to remove unreferenced functions.

--

https://issues.dlang.org/show_bug.cgi?id=19570

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal

--

https://issues.dlang.org/show_bug.cgi?id=19570

--- Comment #9 from Manu <turkeyman@gmail.com> ---
(In reply to Walter Bright from comment #8)
> (In reply to Manu from comment #7)
> >  - emit a copy of the function to each CU it's called from
> 
> But then it's going to be doing code gen for each of them over and over.

Sure.
inline functions are usually very short.
Also, most D applications only have one CU.

> >  - not emit the function to the CU it's declared (unless it's called, as
> > above)
> >  - mark the function with 'internal' linkage (what C++ calls 'static') so
> > it's not in the object's exported symbol table.
> 
> So what if it's in the symbol table?

Multiple modules with the same symbol will have link collisions.
It could be weak, but then you're still inviting 3rd party code to link against
it, which is at odds with it's inline-ness.

> > - symbol is not externally linkable
> 
> Why not?

Because every CU may have its own internal copy.

> > - symbols referenced by an inline function that's never called must NOT cause a link error under any circumstances
> 
> This is a linker problem - the linker is supposed to remove unreferenced functions.

It would be extremely simpler for the problem not to exist at all, and more compatible with existing binary ecosystems without unexpected errors or edge cases.

--