[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:1981
+  // parsed yet).
+  assert(getLangOpts().MicrosoftExt);
+
----------------
RIscRIpt wrote:
> I think I should remove this assertion so this function would be usable without MS ext, on the other hand it would be a noop without MS ext.
I suggest leaving it in solely because use of the function does impose some overhead in the construction of the `std::vector` that is returned. If that overhead can be avoided (I guess by passing the container to populate by reference as a separate argument and then return either the original `ArrayRef` or a new one referencing the populated container), then this could be made a no-op and the checks for `MicrosoftExt` at the call sites could be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914