[PATCH] D85774: [XCOFF][AIX] Enable tooling support for 64 bit symbol table parsing

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:296
+enum SymbolAuxType : uint8_t {
+  _AUX_EXCEPT = 255, ///< Identifies an exception auxiliary entry.
+  _AUX_FCN = 254,    ///< Identifies a function auxiliary entry.
----------------
jasonliu wrote:
> hubert.reinterpretcast wrote:
> > jasonliu wrote:
> > > DiggerLin wrote:
> > > > the name must be consistent with the aix OS file syms.h?
> > > > what about the to change to AUX_EXCEPT . it consistent with our current style. 
> > > The current style in this file seems to be (correct me if I'm wrong):
> > > Use a descriptive name if it's not directly taken from OS header. Otherwise, take the name directly from header and add detailed description/comment to it.
> > > In this case, it follows the latter. Other enums member do not have '_' because OS version do not have it either.
> > The issue here is that the name from the OS header is a reserved name. So by reason of not wanting undefined behaviour, we cannot use the name taken from the OS header. Unfortunately, that means we cannot be consistent in terms of using the name taken from the OS header without switching all the enumerator names to be descriptive and in the LLVM style.
> hmm... Is the namespace `XCOFF` not enough to prevent the undefined behavior happening? Or are we afraid of people just use `using namespace XCOFF` to defeat it?
The practical cause of undefined behaviour in such a case is usually that the instance of the identifier here is misparsed, or otherwise has surprising behaviour, either because it is defined as a macro or is an extension keyword. Whether the name turns out to be in scope elsewhere is not a factor for such mechanisms.


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

https://reviews.llvm.org/D85774