[PATCH] D64014: [Object/ELF.h] - Improve error reporting.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: include/llvm/Object/ELF.h:58
+template <class ELFT>
+static std::string getSecIndex(const ELFFile<ELFT> *Obj,
+                               const typename ELFT::Shdr *Sec) {
----------------
jakehehrlich wrote:
> MaskRay wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > I'm not sure a `static` function here makes sense?
> > > I didn't want to split the definition and declaration.
> > > (because it would require too many lines in a cpp for a template function:
> > > 
> > > ```
> > > template <class ELFT>
> > > std::string getSecIndex(const ELFFile<ELFT> *Obj, ...) { 
> > > ...
> > > }
> > > ...
> > > template void object::getSecIndex<ELF32LE>();
> > > template void object::getSecIndex<ELF32BE>();
> > > template void object::getSecIndex<ELF64LE>();
> > > template void object::getSecIndex<ELF64BE>();
> > > )
> > > ```
> > > 
> > > And so there are two ways to do that correctly I think: either use
> > > `inline` or `static`, otherwise the ODR would be violated.
> > > 
> > > It is probably can be `inline` (like other helpers in this file), I did that change.
> > It makes sense. If static is used, the instances will get internal linkage and defining the template function with the same name in another translation unit will not cause a conflict (ODR violation).
> Agreed. In general if it compiles and can be static you want it to be static (pending some edge cases).
I found this article about "static inline vs inline vs static in C++": https://gist.github.com/htfy96/50308afc11678d2e3766a36aa60d5f75
And it turns out my statement was not fully correct. I assumed that inline implies static, but it is actually not.

Going to change `inline std::string getSecIndex` to `static inline std::string getSecIndex`


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

https://reviews.llvm.org/D64014