[PATCH] D129935: [TableGen] Add a location for a class definition that was forward-declared

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

In D129935#3665997 <https://reviews.llvm.org/D129935#3665997>, @rusyaev-roman wrote:

> In D129935#3665649 <https://reviews.llvm.org/D129935#3665649>, @nhaehnle wrote:
>
>> Right you are, sorry for my confusion. The patch isn't quite right because it overrides the current meaning of the Locs array (which is to track multiclass instantiations), and that can lead to confusion e.g. with `PrintMessage`. I took the liberty of making some adjustments before committing the patch (394a388d140dc9e74178532501cddb558a589398 <https://reviews.llvm.org/rG394a388d140dc9e74178532501cddb558a589398>)
>
> I think the original changes cannot lead to confusion in this case. The behavior for multi-class instantiations has not been affected. The original patch has improved tracking for classes, because before these changes, if we have a declaration (or empty definition) and a definition for some class, we can obtain a location only for its forward-declaration. But the original changes provided a location for the definition as well. What do you think?

My concern here is with the behavior of PrintMessage and friends. The original change would produce misleading output there. WIth the current change, PrintMessage will print the location of the definition (if it is known).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129935/new/

https://reviews.llvm.org/D129935