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
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
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
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/
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
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
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
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
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
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
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
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
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
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:
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
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
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/
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:
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
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
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
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:
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
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
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
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
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
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
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
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
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
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:
-
32 matches
Mail list logo