[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D116774#3227280 <https://reviews.llvm.org/D116774#3227280>, @jrtc27 wrote:

> The fact that va_list is in the std namespace does leak out into __builtin_dump_struct, possibly the odd other place, and of course to external AST consumers.

It can also leak out in funny places like AST dumping (to text, but more interestingly, to JSON). But the one place that may be observable and really matters (that I can think of) are AST matchers that get used by clang-tidy.

> I think it'd make sense to keep ASTContext as putting it in the std namespace for C++ (like it does for Arm, and used to for AArch64) but also have ItaniumMangle override it to the std namespace so that non-C++ gets the right mangling? Otherwise the AST and __builtin_dump_struct won't match the Arm spec.

I think this could make some sense. We typically try to have the AST reflect the reality of the program, and from that perspective, I think I would expect there to be a `std` namespace component for this in the AST if the spec calls for one even when obtaining the type through `stdarg.h` instead of `cstdarg`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116774