[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcd93532dfc45: [MS ABI] Fix C++ mangling references to declarations. (authored by bolshakov-a, committed by efriedma). Repository: rG LLVM Github M

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added a comment. No, I don't have commit access. You could use just `Bolshakov ` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM (Do you have commit access? If not, please specify the name/email you want for the "author" field.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://revie

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + efriedma wrote: > bolshakov-a wrote: > > bolshakov-a wrote: > > > efriedma

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 518226. bolshakov-a added a comment. `SmallVector` instead of `std::stack`; fixing formatting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 Files: clang/lib/AST/MicrosoftMangle.cpp clang/test/Co

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + bolshakov-a wrote: > bolshakov-a wrote: > > efriedma wrote: > > > bolshakov-a

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + bolshakov-a wrote: > efriedma wrote: > > bolshakov-a wrote: > > > efriedma

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + efriedma wrote: > bolshakov-a wrote: > > efriedma wrote: > > > bolshakov-a

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + bolshakov-a wrote: > efriedma wrote: > > bolshakov-a wrote: > > > efriedma wro

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + efriedma wrote: > bolshakov-a wrote: > > efriedma wrote: > > > Weird indent

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + bolshakov-a wrote: > efriedma wrote: > > Weird indentation > I agree. Don't kn

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + efriedma wrote: > Weird indentation I agree. Don't know why clang-format do

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1250 + for (unsigned I = 1, IE = ID->getChainingSize(); I < IE; ++I) +mangleSourceName(""); + Weird indentation Comment at: clang/lib/AST/MicrosoftMangle

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 510345. bolshakov-a added a comment. Minor refactoring. Don't hide function parameter with local variable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 Files: clang/lib/AST/MicrosoftMangle.cpp c

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 510343. bolshakov-a edited the summary of this revision. bolshakov-a added a comment. Mangle array subscripts and base class members in references to subobjects. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 510282. bolshakov-a edited the summary of this revision. bolshakov-a added a comment. Fix mangling of references to subobjects. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 Files: clang/lib/AST/Mi

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added a comment. > The following produces a name collision on MSVC Clang reproduces this buggy behavior. Should it produce an explicit error instead? Or, maybe, some temporary correct algorithm should be found? (It is pretty sad if clang cannot provide new language features for Wind

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 510281. bolshakov-a edited the summary of this revision. bolshakov-a added a comment. Mangling of anonymous unions. @eli.friedman, thank you! > I don't know whether the aforementioned code is acceptable according to the > standard, honestly. It is not a

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma added a comment. The numbers are backreferences of the sort generated by mangleSourceName(), I think. If you nest deeply enough, MSVC stops using them; for example: struct A { union {union { union {union { struct B {struct C {struct D {struct E {struct F {struct G { struct H {s

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1247 + unsigned DiagID = Diags.getCustomDiagID( + DiagnosticsEngine::Error, "cannot mangle anonymous struct/union yet"); + Diags.Report(DiagID); efriedma wrote: >

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added a comment. > Additional levels of nesting append 3's: @3, @33, > etc. Yes, but if you have such a code: struct A { union { union { struct B {}; using T = B; }; }; }; void f(A::T); you have `?f@@YAXUB@@2A@@@Z` for `f`, and for such a c

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma added a comment. > I don't still understand how to mangle nested unnamed tags in general. According to some quick experiments, for the non-virtual case, you mangle a member of an unnamed union it the same way as a regular member, except you stick `@` into the mangling. Additional leve

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:720 +const CXXRecordDecl *RD, const ValueDecl *VD) { + MSInheritanceModel IM = RD->getMSInheritanceModel(); + // ::= rjmccall wrote: > efriedma wrote: > > It's not obvious to

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 508437. bolshakov-a added a comment. Mangle pointer-to-members with conversion. Pointer-to-member-functions with conversions across hierarchy work fine, as I can see. I don't still understand how to mangle nested unnamed tags in general. CHANGES SINCE

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 508436. bolshakov-a added a comment. Add test cases on unspecified inheritance. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 Files: clang/lib/AST/MicrosoftMangle.cpp clang/test/CodeGenCXX/mangle

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

rjmccall added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:720 +const CXXRecordDecl *RD, const ValueDecl *VD) { + MSInheritanceModel IM = RD->getMSInheritanceModel(); + // ::= efriedma wrote: > It's not obvious to me why the inheritance

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

efriedma added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:720 +const CXXRecordDecl *RD, const ValueDecl *VD) { + MSInheritanceModel IM = RD->getMSInheritanceModel(); + // ::= It's not obvious to me why the inheritance model is relevant

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

erichkeane added a comment. Added the code owners here. There doesn't seem to be anything questionable here to me, but I want to make sure someone who really knows what is going on takes a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14638

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a updated this revision to Diff 506413. bolshakov-a added a comment. Rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146386/new/ https://reviews.llvm.org/D146386 Files: clang/lib/AST/MicrosoftMangle.cpp clang/test/CodeGenCXX/m

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

bolshakov-a created this revision. bolshakov-a added reviewers: erichkeane, rnk. Herald added a project: All. bolshakov-a requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Several issues have been discovered and (hopefully) fixed here: - Ref