[PATCH] D107125: [Diagnostic] Split 'qualifier on reference type has no effect' out into a new flag

2021-08-05 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery marked an inline comment as done. lunasorcery added a comment. Yes, that would be "Luna Kirkby " CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107125/new/ https://reviews.llvm.org/D107125 ___ cfe-commits mailing list

[PATCH] D107125: [Diagnostic] Split 'qualifier on reference type has no effect' out into a new flag

2021-08-04 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery updated this revision to Diff 364289. lunasorcery added a comment. Ah, that's much cleaner! I hadn't realized the test framework could do that. I've updated the diff accordingly - your example works perfectly, so I've taken that and ditched the now-unnecessary other test. CHANGES

[PATCH] D107125: [Diagnostic] Split 'qualifier on reference type has no effect' out into a new flag

2021-08-02 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery updated this revision to Diff 363582. lunasorcery added a comment. How does this look? I've got a 'control' test to demonstrate the diagnostic as a child of ignored-qualifiers, as well as a test to demonstrate this diagnostic can be disabled in isolation. CHANGES SINCE LAST ACTION

[PATCH] D106349: [clang-format] respect AfterEnum for enums

2021-07-31 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery added a comment. Copying over the comment I left on D99840 explaining it - my existing change only works because it gets bailed out by further code in TokenAnnotator.cpp. I think your change still is valuable since it's more correct. > Looking at

[PATCH] D107125: [Diagnostic] Split 'qualifier on reference type has no effect' out into a new flag

2021-07-30 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery added a comment. Sure thing - would the suite in /clang/test/SemaCXX/ be the right place to put that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107125/new/ https://reviews.llvm.org/D107125

[PATCH] D107125: [Diagnostic] Split 'qualifier on reference type has no effect' out into a new flag

2021-07-29 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery created this revision. lunasorcery added a reviewer: rsmith. lunasorcery added a project: clang. lunasorcery requested review of this revision. Herald added a subscriber: cfe-commits. This introduces a new flag `ignored-reference-qualifiers` for the existing "'A' qualifier on

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

2021-07-29 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery added a comment. Looking at this again I think the change is flawed. The `InitialToken` is taken *after* the enum token, and additionally the invocation of `ShouldBreakBeforeBrace` is falling through to the `return false;` case every time. As such the call to `addUnwrappedLine()`

[PATCH] D106349: [clang-format] respect AfterEnum for enums

2021-07-29 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery added a comment. Note also there's significant overlap with the now-committed D99840 , though that's missing the change inside ShouldBreakBeforeBrace(). In retrospect I'm a little confused as to why D99840 appears

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

2021-07-27 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery updated this revision to Diff 362220. lunasorcery added a comment. Sorry it took me so long to get back to this - fell off my radar somewhat. I've updated the release notes, I think it's ready to commit now? I lack commit rights (first time contributor!) so for attribution it'd be

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

2021-04-03 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery created this revision. lunasorcery added reviewers: djasper, klimek. lunasorcery added a project: clang-format. lunasorcery requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously, with AllowShortEnumsOnASingleLine disabled,