Page MenuHomePhabricator

[tablegen] Replace strconcat, listconcat by concat
Needs ReviewPublic

Authored by johannes on Dec 4 2017, 6:07 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Previously, the name concat was used by the operator !con. These
usages are renamed to just con, so that concat can be used by the new operation
which subsumes strconcat, listconcat and also works for code fragments. The old
!strconcat and !listconcat are just aliases for !concat.

Event Timeline

johannes created this revision.Dec 4 2017, 6:07 AM
johannes updated this revision to Diff 125324.Dec 4 2017, 6:10 AM

adhere to the column limit

I think there should be some documentation for this new TableGen command.

johannes updated this revision to Diff 126537.Dec 12 2017, 5:56 AM

add documentation + test

hfinkel added inline comments.
lib/TableGen/Record.cpp
766

Maybe call this ConcatStringLikeInits or ConcatInits? It should be renamed now that it is not specific to StringInits.

johannes updated this revision to Diff 127306.Dec 18 2017, 12:55 AM

remove codeconcat, make strconcat return code if all arguments are

johannes retitled this revision from [tablegen] Add !codeconcat operator which works like !strconcat to [tablegen] Make strconcat return code if all arguments are code.Dec 18 2017, 12:56 AM
johannes edited the summary of this revision. (Show Details)

I use a different approach now, WDYT?

lib/TableGen/Record.cpp
766

yeah, makes sense, done

I use a different approach now, WDYT?

I'm not sure I like this. Before it was clear: !strconcat is for strings, !codeconcat was for code. Having !strconcat also work on code, and return something which isn't a string, seems unfortunate. And we also have !listconcat. If we want to unify things, I think it would make sense to add !concat (which works on all of those things). [If you do this, then also support !strconcat and !listconcat as aliases to be nice to out-of-tree backends].

From a user's perspective, I got the impression that code [{}] is just a handy shortcut for multiline strings and I was quite surprised that they are internally actual different datatypes. So for me, changing the semantic of !strconcat would remain rather intuitive.

I use a different approach now, WDYT?

I'm not sure I like this. Before it was clear: !strconcat is for strings, !codeconcat was for code. Having !strconcat also work on code, and return something which isn't a string, seems unfortunate. And we also have !listconcat. If we want to unify things, I think it would make sense to add !concat (which works on all of those things). [If you do this, then also support !strconcat and !listconcat as aliases to be nice to out-of-tree backends].

Oh right, thanks, implementing !concat plus the aliases for the old functions seems to be the cleanest solution, assuming that this change won't cause too much confusion in existing code. I think it should be fine but I don't know how many people are using these operators.

johannes updated this revision to Diff 128220.Dec 27 2017, 2:51 AM
johannes retitled this revision from [tablegen] Make strconcat return code if all arguments are code to [tablegen] Replace strconcat, listconcat by concat.
johannes edited the summary of this revision. (Show Details)

use generic concat

If this will be approved I can also update existing usages of strconcat to concat