[PATCH] D79984: [yaml2obj] - Add a technical prefix for each unnamed chunk.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/section-link.yaml:32
+# ERR:      error: unknown section referenced: '.unknown1' by YAML section '.foo'
+# ERR-NEXT: error: unknown section referenced: '.unknown2' by YAML section ' [unnamed, index #2]'
+# ERR-NEXT: error: unknown section referenced: '.unknown3' by YAML section '.bar'
----------------
grimar wrote:
> MaskRay wrote:
> > Can we omit `unnamed` and use `'' (index 2)`?
> We can omit/add whatever we want into `[ ... ]` part.
> 
> This patch works because it automatically adds unique suffixes. What we use for
> describing sections/symbols with the same name already:
> 
> ```
>   - Name: '.foo [1]'
>     Type: SHT_PROGBITS
>   - Name: '.foo [2]'
>     Type: SHT_PROGBITS
> ```
> 
> I.e. we can add any `[ any text here ]` as suffix to a section name and the existent logic will
> handle it properly. But we can't switch `[]` to `()` without doing additional changes.
> 
> It could be `' [index #2]'`, but not `'' [index 2]` nor `'' (index 2)`.
> 
> It can be possible to change (add an additional name parsing of a section name) in 
> `toSectionIndex` to improve the message reported, but I am not sure it worth that additional complexity:
> 
> ```
> template <class ELFT>
> unsigned ELFState<ELFT>::toSectionIndex(StringRef S, StringRef LocSec,
>                                         StringRef LocSym) {
> ....
> //// Add a code to parse LocSec/LocSym here.
> 
>   if (!LocSym.empty())
>     reportError("unknown section referenced: '" + S + "' by YAML symbol '" +
>                 LocSym + "'");
>   else
>     reportError("unknown section referenced: '" + S + "' by YAML section '" +
>                 LocSec + "'");
>   return 0;
> }
> ```
OK, this name should be fine. But why doesn't `'' [index 2]"` work? (I am actually ok with either one but `#2` not working seems strange to me.)


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

https://reviews.llvm.org/D79984