[PATCH] D156420: [TableGen] Add `!dump` and `dump`.

Thu Aug 10 00:46:45 PDT 2023

fpetrogalli abandoned this revision.
fpetrogalli added a comment.

In D156420#4573789 <https://reviews.llvm.org/D156420#4573789>, @tra wrote:

> In D156420#4572483 <https://reviews.llvm.org/D156420#4572483>, @fpetrogalli wrote:
>
>> However, I have the following considerations:
>>
>> 1. `!dump` is a debugging facility that is not supposed to be used in production code
>
> I don't think it rules out keeping tablegen design clean.
>
>> 2. `!format` is a formatting facility that is supposed to be used in production code for generating strings out of values.
>
> That part I'm fine with.
>
> All I'm saying is that !dump() accepting a description is something that tablegen will be able to do by using a function or lambda, once D148915 <https://reviews.llvm.org/D148915> lands, which would render the description part superfluous.
>
> So, `!format` would handle conversion of objects to printable strings, 
> `!dump` would be responsible for printing out whatever it gets as an argument.
> functions would allow users to compose whatever is needed from the primitive components.
>
>> In particular, the use of 1 requires to be able to quickly find in the output of tablegen that prints the variable. 
>> It might be just my personal preference, but think it is more efficient to just type `dump("a=", myvar)` than `!dump(!format("a=...", myvar))`, especially given the fact that such use case is there as a one-off, just for debugging.
>
> IMO, it's a moot point, as `!dump(!format("a=...", myvar))`, can be wrapped in a function, and become exactly the `dump("a=", myvar)` you're looking for.
> The difference is that you propose to hardcode `dump(description, value)`, while I'm suggesting to leave it up to the user to decide what's the best way for them. E.g. they may not want a description, or they may want to accept multiple values.

Let's go with D157492 <https://reviews.llvm.org/D157492>, then :)

I will update it soon.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156420