[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7ae1b4a0ce9c: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for… (authored by rsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Okay, if the basic idea is to avoid anything that would need the linkage to be computed, that seems like exactly the right goal. I know one of the awful ways to get an early reference to the anonymous type is to define a typedef of like a pointer to it, then use that i

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Ugh, you mentioned template argument lists and made me realize we can still reach problem cases that way. And with array bounds. :( I've added back in a diagnostic for the case where we compute the linkage too early for a "C-like" struct. Hopefully people won't hit it to

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 243057. rsmith marked an inline comment as done. rsmith added a comment. Add back a diagnostic for remaining cases that the new langauge rule doesn't handle. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74103/n

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, LGTM, assuming that we do indeed want to ban friends. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74103/new/ https://reviews.llvm.

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 243048. rsmith marked 3 inline comments as done. rsmith added a comment. Extend assert comment to suggest likely remediations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74103/new/ https://reviews.llvm.org/D7

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:4394 +isa(D)) + continue; + rsmith wrote: > rjmccall wrote: > > This is essentially part of the next paragraph. > > > > I believe `friend` declarations also do not declare "memb

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:4394 +isa(D)) + continue; + rjmccall wrote: > This is essentially part of the next paragraph. > > I believe `friend` declarations also do not declare "members", under the same >

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 243022. rsmith marked 4 inline comments as done. rsmith added a comment. Updates based on review comments from @rjmccall. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74103/new/ https://reviews.llvm.org/D74103

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 243023. rsmith added a comment. Add test coverage for the 'friend' case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74103/new/ https://reviews.llvm.org/D74103 Files: clang/docs/ReleaseNotes.rst clang/inc

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Oh thank goodness, they finally fixed this. Comment at: clang/lib/Sema/SemaDecl.cpp:4365 + // C++ [dcl.typedef]p9: [P1766R1] + // An unnamed class with a typedef name for linkage purposes shall not + if (RD->isInvalidDecl()) Start

[PATCH] D74103: Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

2020-02-05 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. Due to a recent (but retroactive) C++ rule change, only sufficiently C-compatible classes are permitted to be given a typedef name for linkage purposes. Add an en