[PATCH] D42475: [ELF] Add warnings for various symbols that cannot be ordered

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

My thoughts below. Though probably part about changes in Driver.cpp 
may conflict with previous Rui's comment.

Rui, did you mean you want to compute priority in driver ? I realize why you did
that in your version, but since it was decided to keep parts of logic
in writer, it seems to be more appropriate place for doing that.
I assume you meant to report "specified multiple times" there, but 
leave the rest to `buildSectionOrder` ?



================
Comment at: ELF/Config.h:77
+  bool Present;
+};
+
----------------
I do not think this struct should be here.
I believe in Driver.cpp you want to report "specified multiple times" error,
but `Priority` and `Present` are things that you need only once in
`buildSectionOrder` and so them probably should be computed, used
and then forgotten right there.


================
Comment at: ELF/Config.h:86
   llvm::CachePruningPolicy ThinLTOCachePolicy;
+  llvm::MapVector<StringRef, SymbolOrderEntry> SymbolOrderingFile;
   llvm::StringMap<uint64_t> SectionStartMap;
----------------
So I think it can be reverted to
```
std::vector<llvm::StringRef> SymbolOrderingFile;
```

And then you could for example use local `DenseSet<llvm::StringRef>`
in `getSymbolOrderingFile` to check and report duplicates.


================
Comment at: ELF/Driver.cpp:603
+  Optional<MemoryBufferRef> MB = readFile(Filename);
+  MapVector<StringRef, SymbolOrderEntry> Ret;
+  if (!MB)
----------------
This can be:

```
if (!MB) 
  return {};

MapVector<StringRef, SymbolOrderEntry> Ret;
...
```


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42475