Break dependency of expression.d on gluelayer.d

[RazvanN7]

No description provided.

[dlang-bot]

Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#15786"

[dkorpel]

Long term I want to to get rid of storing backend symbols inside frontend classes altogether

[ibuclaw]

Long term I want to to get rid of storing backend symbols inside frontend classes altogether

We tried internal AA for each kind of backend symbol, but that was denied for being too slow (without benchmarking, but I think it is a given that anything is slower than direct pointer access).

[ibuclaw]

I think this is incomplete. There's also typedefs to deal with.

Regardless, I don't think this is as small a change for C++ codebases as it is for D. So hold fire.

[dkorpel]

I thought Symbol was dmd-only, so it wouldn't affect other compilers?

[RazvanN7]

Symbol seems to be an alias to a tree_node in gcc. I am expecting that all you need to do on the GCC side is to replace all occurences of sym to (tree_node*)sym.

[RazvanN7]

@dkorpel the gluelayer has a version(IN_GCC) that alias Symbol to an extern gcc symbol (tree_node). Still, I don't see how this makes things complicated for gdc.

[thewilsonator]

Regardless, I don't think this is as small a change for C++ codebases as it is for D. So hold fire.

I highly doubt it. At most it requires the insertion of a couple of pointer casts.

LDC #ifs out the use of Symbol in module.h so the typedefs are a non-issue for us. Indeed dmd.gluelayer does not define Symbol with version (IN_LLVM), at all. The usage here is void* in D and llvm::Type* in the header file.

It has all of 4 uses which would add 4 pointer casts.

Also this PR affects only StructLiteralExp it doesn't touch any other uses of Symbol

[ibuclaw]

@thewilsonator don't auto-merge after 15 minutes of the PR being open please. Not everyone affected is in the same timezone, and can respond in short notice. :-)

[RazvanN7]

Long term I want to to get rid of storing backend symbols inside frontend classes altogether

Agreed, however, apart from AAs, I don't really have an idea on how to do that.

[ibuclaw]

Long term I want to to get rid of storing backend symbols inside frontend classes altogether

Agreed, however, apart from AAs, I don't really have an idea on how to do that.

There's the (possibly ill-defined) Compiler interface that supposed to let compiler vendors do things in an agnostic way. Compiler back-end types could be moved there:

struct Backend {
  version (MARS)
  union {
    Symbol* _sym;
    TYPE* _ctype;
    //... 
  }
  auto sym() { return _sym; }
  void sym(s) { _sym = s; }
}

but that just changes the names from one thing to another.

[RazvanN7]

@ibuclaw how do we move forward from here? This is the last of expression.d's unwanted dependency.

[thewilsonator]

@RazvanN7 please rebase

Not a disruptive change to C++ users, only requires a few pointer casts.

[ibuclaw]

Reverting because we agreed the proper way last DLF meeting.

[thewilsonator]

... which is?

[ibuclaw]

... which is?

You would already know if you didn't dismiss and merge PRs within 5 minutes.

Ask next time please.