This is an archive of the discontinued LLVM Phabricator instance.

[llvm-rc] Allow optional commas between the string table index and value
ClosedPublic

Authored by mstorsjo on May 6 2018, 2:34 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.May 6 2018, 2:34 PM
zturner accepted this revision.May 7 2018, 10:01 AM
zturner added inline comments.
test/tools/llvm-rc/Inputs/tag-stringtable-basic.rc
9 ↗(On Diff #145415)

Can you put a comment above this that says "commas are optional, so we make sure to test both cases"?

This way someone editing this test in the future knows to preserve the commas.

Alternatively, you could break this out into a separate test called tag-stringtable-commas.rc

This revision is now accepted and ready to land.May 7 2018, 10:01 AM
mstorsjo added inline comments.May 7 2018, 10:56 AM
test/tools/llvm-rc/Inputs/tag-stringtable-basic.rc
9 ↗(On Diff #145415)

Sure, I'll add a comment.

amccarth accepted this revision.May 7 2018, 11:21 AM

As I recall, commas may be optionally allowed in several places besides string tables. We'll have to check that and, if necessary, address in a subsequent patch.

This revision was automatically updated to reflect the committed changes.