[PATCH] D82549: [AIX][XCOFF] parsing xcoff object file auxiliary header

[Digger Lin via Phabricator via llvm-commits]

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


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:605
+
+#define PrintAuxMember32(H, S, T)                                              \
+  if (offsetof(XCOFFAuxiliaryHeader32, T) +                                    \
----------------
jhenderson wrote:
> DiggerLin wrote:
> > jhenderson wrote:
> > > Could this not just be a lambda? That would play friendlier with IDEs if nothing else.
> > the T is a member of class XCOFFDumper.cpp , I do not think C++11 support a template lambda.
> Not sure I quite follow, but note that LLVM supports C++14 and `auto` can be used to much the same effect, if I'm not mistaken. I believe something like the following might work (note: I haven't tested, and am not particularly familiar with auto lambdas).
> ```
> enum PrintStyle { Hex, Number };
> auto PrintAuxMember32 = [&](PrintStyle P, StringRef S, auto *T) {
>   ptrdiff_t Offset = T - AuxHeader;
>   if (Offset + sizeof(*T) <= AuxSize) {
>     switch(P) {
>     case Hex:
>       W.printHex(S, *T);
>       break;
>     case Number:
>       W.printNumber(S, *T);
>       break;
>     }
>   } else if (Offset < AuxSize) {
>     PartialFieldOffset = Offset;
>     PartialFieldName = S;
>   }
> }; 
> 
> PrintAuxMember32(Hex, "Magic", &AuxHeader->AuxMagic);
> PrintAuxMember32(Hex, "Version", &AuxHeader->Version);
> // etc...
> ```
> You might be able to avoid the enum using some sort of std::bind call or similar, which would be nicer, but not essential.
> 
> Even without that, I think you should factor out the repeated `offsetof` calls into a variable, and use more meaningful variable names rather than `H`, `S` and `T`, just like you would for any function.
there is an article about the auto for c++14. https://solarianprogrammer.com/2014/08/21/cpp-14-auto-tutorial/

G++ 4.9.1 also supports unconstrained generic functions, basically you can use auto in a function parameter list, so the above code can be further simplified as :

 1     auto& add_one(auto& v) {
 2         for(auto& it : v) {
 3             it += 1;
 4         }
 5         return v;
 6     }
 7 
 8     void multiply_by_two(auto& v) {
 9         for(auto& it : v) {
10             it *= 2;
11         }
12     }
unfortunately, this didn’t entered in the final C++14 standard. It is expected to be added later to the standard as a technical report.

I test the code in the clang, 
the 'auto' not allowed in function prototype

@jhenderson 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82549