[PATCH] D63596: [yaml2obj/obj2yaml] - Allow having the symbols and sections with duplicated names.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/obj2yaml/duplicate-symbol-and-section-names.test:4
+## names and that the suffixes produced for each section and
+## symbol reflects the index of duplication.
+
----------------
grimar wrote:
> jhenderson wrote:
> > reflects -> reflect
> > 
> > I'm not sure I understand what "index of duplication" means. Probably sufficient to say that the suffixes are unique (it really doesn't matter how they are unique e.g. with index).
> > I'm not sure I understand what "index of duplication" means.
> 
> Order number of the duplication? I.e. `[1`] below means this is the first duplication for `localfoo`,
> `[2]` means the second and so on.
> 
> ```
> # CASE1-NEXT:   - Name: localfoo
> # CASE1-NEXT:   - Name: 'localfoo [1]'
> # CASE1-NEXT:   - Name: 'localfoo [2]'
> ```
> 
> > Probably sufficient to say that the suffixes are unique
> 
> They are not. They were unique in the first versions of this patch,
> but now we can have both `localfoo [1]` and `localbar [1]` for example.
> 
> > (it really doesn't matter how they are unique e.g. with index).
> 
> It does not matter for `yaml2obj`. but here we testing `obj2yaml` output,
> which has a special logic to produce 
> `localfoo, 'localfoo [1]', localbar, 'localbar [1]'`
> instead of `localfoo, 'localfoo [1]', localbar, 'localbar [2]'`
> 
> That is what I tried to explain in the comment.
> 
> What about the following?
> "and that the suffixes produced for each section and symbol shows the order number
>  of the duplication"
To be clear, I did eventually understand it, but the wording isn't how I'd word things.

I don't think it really matters what obj2yaml produces, as long as each different section with the same name has different suffixes in the output, so references to the order are irrelevant. As far as I am concerned, obj2yaml could write them as "localbar [bear]", "localbar [cat]", "localbar [dog]" etc. Ideally, I'd have this test simply check that the numbers between '[' and ']' are different to each other, for matching section/symbol names, but that's not practical.

How about "and produces same-named sections and symbols with distinguishing suffixes."?




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

https://reviews.llvm.org/D63596