[PATCH] D67924: [Sema] Fix the atomic expr rebuilding order.

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 23 10:47:40 PDT 2019


erichkeane added a comment.

It kinda stinks that we have to do this at such a late step.  I'd have much preferred doing this as a part of the rebuild, but it appears that the 'form' takes quite a bit to calculate.  Also, I'd likely have preferred that the initial reordering happened in codegen instead of this early, though perhaps thats too large of a change to do here.

I think then we're stuck with this reordering step.

That said, I'm not a fan at all of doing this parameter as a bool, particularly one with an ambiguous name.  Can you introduce an enum like "enum class AtomicArgumentOrder { API, AST}" that makes it more clear?

Additionally, ReArgs is a confusing name, how about APIOrderedArgs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67924





More information about the cfe-commits mailing list