compnerd marked 2 inline comments as done. compnerd added inline comments.
================ Comment at: lib/AST/MicrosoftMangle.cpp:988 + if (const auto *RD = dyn_cast<RecordDecl>(DC)) + mangleName(RD); + else ---------------- efriedma wrote: > The call to mangleName() looks a little weird... I would have expected a call > to mangleUnqualifiedName or something like that. (If there's some reason for > the asymmetry, a brief comment explaining it would be fine.) Accounting for the parameter like we do for lambdas makes this unnecessary. ================ Comment at: lib/AST/MicrosoftMangle.cpp:1008 + if (const auto *ND = dyn_cast<NamedDecl>(MC)) + mangleUnqualifiedName(ND); + ---------------- efriedma wrote: > This isn't quite the same thing the Itanium mangling does... > > Itanium has special cases for in-class initializers and default arguments. > Using the name is fine for in-class initalizers, but it doesn't really work > for default arguments: arguments aren't required to have names. Consider, > for example: > > ``` > extern int e(void); > class C { > void m(int = ^{ static int i = e(); return ++i; }(), > int = ^{ static int i = e(); return ++i; }()); > }; > ``` Added a test case and handled it. ================ Comment at: test/CodeGenCXX/msabi-blocks.cpp:90 +} + ---------------- efriedma wrote: > The Itanium ABI document lists five cases where the mangling is externally > visible. I think this is missing a testcase for the "initializers of > nonspecialized static members of template classes" case. (Something like > "template<typename T> class X { static T foo; }; template<typename T> T > X<T>::foo = ^{static int i = e(); return ++i;}();".) This is currently broken even under the itanium scheme. I think that doing that in a follow up is reasonable. ``` template <typename T> struct s { static T i; }; template <typename T> T s<T>::i = ^{ static T i = T(); return i; }(); template class s<int>; template class s<short>; ``` Repository: rL LLVM https://reviews.llvm.org/D34523 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits