[llvm] clang crash assigning to a global named register variable #109778 (PR #113105)

[David Spickett via llvm-commits]

DavidSpickett wrote:

Hi, I think I was the one who filed this issue in the first place. https://github.com/llvm/llvm-project/issues/109778.

We generally don't put the number of the issue being fixed in the title. Instead you can put `Fixes #<issue number>` in the PR description. https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

(and it is fine to do this even if you're not 100% sure that it does, because we can always edit the message later)

> I am not sure if the fix is correct. I tried to analyze the logic and found potential issue in the logic and corrected it. 

PR descriptions should explain the "why" of the change, at least as well as it is understood at the time of submission.

So for this change, what do you think the logic was doing, why was that incorrect and what did you change to fix that?

Not trying to trick you here, I don't know how the code works here either, first time reading it :) But without an attempt at an explanation, we cannot discern between a "fix", and simply avoiding the specific error that was reported. The latter may in fact make everything else worse if we do not think about it properly.

> Because of this, two testcases;

Two testcases what? Do you mean you added those or that they failed because of the change, not sure. I assume you mean they failed, because I don't see any new tests added.



https://github.com/llvm/llvm-project/pull/113105