[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/include/clang/AST/MangleNumberingContext.h:57-58
+
+  /// Has device mangle numbering context.
+  virtual bool hasDeviceMangleNumberingContext() const { return false; }
+
----------------
It would be nicer if there were a single entry point that does the right thing for all mangling contexts.


================
Comment at: clang/lib/AST/MicrosoftCXXABI.cpp:67-68
 
+class MSHIPNumberingContext : public MangleNumberingContext {
+  MicrosoftNumberingContext HostCtx;
+  std::unique_ptr<MangleNumberingContext> DeviceCtx;
----------------
I know it's inheritance instead of composition, but maybe the simple thing to do here would be to inherit from the MicrosoftMangleNumberingContext, and then implement an override for getDeviceManglingNumber that delegates to the Itanium one.


================
Comment at: clang/lib/AST/MicrosoftCXXABI.cpp:114
 
+  std::unique_ptr<MangleContext> Mangler;
+
----------------
This is actually the device's mangling context, and it's an Itanium mangling context. I think it needs comments and a more descriptive variable name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322