This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG29444f0444c6: [modules] Merge ObjC interface ivars with
anonymous
vsapsai added a comment.
Thanks everyone for the feedback! Especially valuable is opinion on making
`ObjCContainerDecl` a lexical decl context. As discussed earlier, not many
people can provide feedback on Objective-C-related code and I'm still
responsible for any problems caused by this
vsapsai added a reviewer: benlangmuir.
vsapsai added a subscriber: benlangmuir.
vsapsai added a comment.
I've run clang with this change on a bunch of code (especially Objective-C
code) and there were no regressions.
Also adding @benlangmuir to reviewers as people mentioned he was doing some
vsapsai updated this revision to Diff 417468.
vsapsai added a comment.
Fix bug with anonymous enum constant lookup in C++.
Turns out the lookup was broken by using `ObjCInterfaceDecl` as a semantic decl
context for anonymous enums. Fixing that fixed the lookup.
Also added in tests checking
vsapsai planned changes to this revision.
vsapsai added inline comments.
Herald added a project: All.
Comment at: clang/test/Modules/merge-anon-record-definition-in-objc.m:5
+// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks %t/test.m
-Wno-objc-property-implementation
rsmith added a comment.
The modules side of this looks good to me, and logically changing the lexical
decl context in an interface to be that interface makes a lot of sense, but I
agree with @rjmccall that it's hard to predict what the consequences of that
change might be. Can you also test
vsapsai added inline comments.
Comment at: clang/test/Modules/merge-anon-record-definition-in-objc.m:5
+// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks %t/test.m
-Wno-objc-property-implementation -Wno-incomplete-implementation \
+// RUN:-fmodules
rjmccall added a comment.
This is one of those patches that's difficult to review because it's so hard to
foresee the consequences of changing the concepts. That said, I think the
basic idea here seems reasonable.
Comment at:
vsapsai added a comment.
Ping.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118525/new/
https://reviews.llvm.org/D118525
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vsapsai updated this revision to Diff 406977.
vsapsai added a comment.
Herald added a project: clang.
Rebase and address clang-format comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118525/new/
https://reviews.llvm.org/D118525
Files:
vsapsai added a comment.
@akyrtzi I believe you were working on the code responsible for decl contexts
at some point. I'm not sure you'll be able to review the change. Can you
recommend someone else knowledgeable with this code?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
vsapsai created this revision.
vsapsai added reviewers: ahatanak, rsmith.
vsapsai added a project: clang-modules.
Herald added a subscriber: ributzka.
vsapsai requested review of this revision.
Without the fix ivars with anonymous types can trigger errors like
> error: 'TestClass::structIvar'
12 matches
Mail list logo
