Add bool gototarget to CaseStatement and DefaultStatement

[JohanEngelen]

(First commit is a small cleanup in SwitchStatement.semantic.)

Add bool gototarget to CaseStatement and DefaultStatement that is set to true when the switch contains a goto case targetting that CaseStatement (or DefaultStatement)
gototarget is used in LDC for PGO.
The variable is not currently used in DMD, but may be useful in the future, e.g. for control flow analysis.

Note that I also had to create SwitchStatement.hasGotoDefault variable. I did not find a satisfactory way to avoid creating that variable.

[ibuclaw]

There's something very iffy about this PR.

[ibuclaw]

I'm pretty certain you can achieve what you are doing without this addition. I'm just going off memory, but the frontend validates all goto targets, including forward references, so this information should be available in ast by the time you reach the backend.

[dnadlinger]

It's true that there might be a better way of doing this. In fact, that was also my reaction when reading the LDC PR diff. However, I can't say from memory (or having a glance at the code) how to efficiently do it, hence I suggested just raising a PR here. My hope is that somebody who knows a better solution will use it as an argument for turning this PR down. ;)

[JohanEngelen]

Of course it is in the AST: when you hit a CaseStatement, go up to the SwitchStatement and traverse all of the code in all cases, and find whether there is a "goto case" for your particular CaseStatement.
The goal here is not to do that whole traversal, but store it instead during semantic analysis.

[JohanEngelen]

Compare with LabelStatement.breaks

[dnadlinger]

Ping @WalterBright @yebblies.

[WalterBright]

I don't think this should be added unless there is an imminent plan for using it.

[dnadlinger]

It is being used in LDC right now. This PR is just the same as #5577 – inlinedNestedCallees there is only going to be used by DMD, at least in the short term. It's true that this addition is a bit clunky, but I couldn't find a good way of regenerating the information after the fact except for recursively walking the full AST again.

In the bigger picture, the comparison to #5577 highlights one major reason for the considerable diff between the DMD and LDC frontends right now: The AST output is very much tailored to exactly what the DMD glue layer needs, whereas we can't just add whatever is convenient for LDC to the upstream source.

[ibuclaw]

Maybe we should have something in place to push information relevant for a particular backend? Though I would be surprised if you can't go this in llvm ast. I'm using gcc language-specific structures (opaque in the backend) to store more information away to rely less on the Frontend into gcc trees. Soon I won't even be using the frontend to cache backend pointers.

[UplinkCoder]

@JohanEngelen please close.

[ibuclaw]

In the meantime - I've filed an issue here: https://issues.dlang.org/show_bug.cgi?id=17166

[andralex]

@JohanEngelen status? should I just close?

[JohanEngelen]

I don't understand why this hasn't been merged. It adds analysis functionality to the frontend that is useful and used by at least one backend. For me, it's been a nice example of why LDC needs its own frontend to progress, but manpower is limited so I don't push for it.

By now the PR needs work to be mergable, and I won't do it.

[andralex]

@JohanEngelen sorry about that! @WalterBright think you could take this over?

[andralex]

or @RazvanN7

[JinShil]

From @WalterBright

I don't think this should be added unless there is an imminent plan for using it.

From @ibuclaw

There's something very iffy about this PR.
I'm pretty certain you can achieve what you are doing without this addition.

From @JohanEngelen

By now the PR needs work to be mergable, and I won't do it.

There doesn't appear to be much support for this and @JohanEngelen is clearly not interested in pursuing it. An issue was filed at https://issues.dlang.org/show_bug.cgi?id=17166 to capture the problem. Therefore, I'm closing this.

[ibuclaw]

To capture the essence of my tone better:

• There are useless unused fields in the frontend that only have a meaning for DMD.
• LDC adds their own fields on top of this in their own fork.
• GDC doesn't add any fields to the frontend, rather it stores that extra heuristic information into backend trees during the codegen pass.

There's no excuse why DMD can't follow my lead here and remove their own boilerplate.

[WalterBright]

Added to Phantom Zone because indeed this is useful for doing flow analysis (the CSX flows). I don't know yet if it is the right answer, but it is good as a starting point.