[PATCH] D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable.

logan-5 created this revision. logan-5 added reviewers: rsmith, dblaikie. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is a fix for rG864949 which only disabled default construction and assignment for lambdas with capture-defaults, where the C++2a draft disables th

[PATCH] D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable.

logan-5 added a comment. *ping* Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64058/new/ https://reviews.llvm.org/D64058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/

[PATCH] D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable.

logan-5 added a comment. I believe I need someone to commit it for me... brand new to all this and unsure, frankly. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64058/new/ https://reviews.llvm.org/D64058 ___ cfe-co

[PATCH] D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable.

logan-5 added a comment. I had the exact same thought about factoring out this and the function pointer conversion condition. I didn't bother, but I agree it could be a useful function to have. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64058/new/ https://re

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 added a comment. Pinging this. I believe all feedback from @EricWF was addressed back in my update to the patch in March. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72282/new/ https://reviews.llvm.org/D72282 ___ cfe-commits mail

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 added a comment. In D82728#2161152 , @xbolva00 wrote: > Is it possible to emit fixit note with "override" ? This is a good idea, though unfortunately (after eyeballing the implementation of `modernize-use-override` in clang-tidy (UseOverrideChec

[PATCH] D84213: [clang-tools-extra] Disable -Wsuggest-override for unittests/

logan-5 created this revision. logan-5 added reviewers: aaron.ballman, klimek, alexfh, juliehockett, sammccall. logan-5 added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. I added `-Wsuggest-override` to the LLVM build earlier today, an

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 created this revision. logan-5 added a reviewer: mgorny. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. Yesterday I enabled `-Wsuggest-override` in the main LLVM build and have been fighting with the -Werror bots ever since. The key culprits making this difficult

[PATCH] D84213: [clang-tools-extra] Disable -Wsuggest-override for unittests/

This revision was automatically updated to reflect the committed changes. Closed by commit rGfa42b7cf2949: [clang-tools-extra] Disable -Wsuggest-override for unittests/ (authored by logan-5). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84213/new/

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2164537 , @Quuxplusone wrote: > For GTest and GMock specifically, it would be a good medium-term idea to try > to upstream patches into those libraries. I did eventually have success > upstreaming fixes for `-Wdeprecate

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. Committing this now to fix the bots, as per discussion in https://reviews.llvm.org/D84126. I'll happily revert and approach it differently later if anyone requests that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84244/

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2165592 , @dblaikie wrote: > Looks OK (for future reference, this sort of stuff should've been cleaned up > before enabling the flag so as to avoid this kind of hurry/breakage/etc) Loud and clear. I honestly thought I

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. Thanks for reverting--I agree that that's the right move at this point. Pretty much totally out my depth here, and I don't have a way of debugging the Windows issue, so I'm not sure how to proceed. I'm tempted to just let this whole thing rest for now and maybe try agai

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2167625 , @hans wrote: > I don't really know why this doesn't happen with other warning flags, but I > think it would be better to add flags like this with add_compile_options > rather than add_compile_definitions. I im

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2167663 , @dblaikie wrote: > Sometimes it's OK to commit stuff an see how it goes on the buildbots - doing > so out of peak times and with the intention of rolling back > quickly/immediately if the buildbots report brea

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2169142 , @hans wrote: > Looks good to me so far. We haven't tried doing any full builds yet, but I > checked for the specific problem I hit yesterday (building > ClangApplyReplacementsTests) and that's working fine now

[PATCH] D84554: Use INTERFACE_COMPILE_OPTIONS to disable -Wsuggest-override for any target that links to gtest

logan-5 created this revision. logan-5 added a reviewer: labath. Herald added subscribers: llvm-commits, cfe-commits, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, jpienaar, rriddl

[PATCH] D84554: Use INTERFACE_COMPILE_OPTIONS to disable -Wsuggest-override for any target that links to gtest

logan-5 added a comment. In D84554#2175012 , @labath wrote: > Could you elaborate on the lldb issue? I'd like to take a look at that... It looks like lldb/unittests/TestingSupport/CMakeLists.txt and lldb/unittests/TestingSupport/Symbol/CMakeLists.txt bo

[PATCH] D84554: Use INTERFACE_COMPILE_OPTIONS to disable -Wsuggest-override for any target that links to gtest

This revision was automatically updated to reflect the committed changes. Closed by commit rGa52aea0ba624: Use INTERFACE_COMPILE_OPTIONS to disable -Wsuggest-override for any target that… (authored by logan-5). Changed prior to commit: https://reviews.llvm.org/D84554?vs=280564&id=280933#toc Re

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 updated this revision to Diff 275928. logan-5 marked 6 inline comments as done. logan-5 added a comment. Addressed some feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82728/new/ https://reviews.llvm.org/D82728 Files: clang/inclu

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 added a comment. Glad this is generating some discussion. For my $0.02, I would also (obviously) love to be able to enable this warning on all the codebases I work on, and this patch was born out of a discussion on the C++ Slack with another user who had found this warning very useful i

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 added a comment. In D82728#2135149 , @dblaikie wrote: > Is the implementation you're proposing fairly consistent with GCC's? Run it > over any big codebases to check it warns in the same places GCC does? This patch has the same behavior as `-Wsu

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 updated this revision to Diff 276162. logan-5 added a comment. clang-formatted the diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82728/new/ https://reviews.llvm.org/D82728 Files: clang/include/clang/Basic/DiagnosticGroups.td clan

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 marked 2 inline comments as done. logan-5 added a comment. In D82728#2137021 , @dblaikie wrote: > I think it might be nice to make the -Wno-inconsistent-missing-override > -Wsuggest-override situation a bit better (by having it still do the same

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 updated this revision to Diff 276187. logan-5 added a comment. Addressed minor feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82728/new/ https://reviews.llvm.org/D82728 Files: clang/include/clang/Basic/DiagnosticGroups.td clan

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 added a comment. In D82728#2137492 , @dblaikie wrote: > Oh, yep, there's a way - it's usually used for performance-costly warnings, > to not spend the resources computing the warning if it's disabled anyway. Wow, thanks--overlooked that in a big

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 updated this revision to Diff 276248. logan-5 added a comment. Fall back to -Wsuggest-override if -Winconsistent-missing-override is disabled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82728/new/ https://reviews.llvm.org/D82728 Files:

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang/test/SemaCXX/warn-suggest-destructor-override:1 +// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-destructor-override + dblaikie wrote: > Does GCC have sug

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang/test/SemaCXX/warn-suggest-destructor-override:1 +// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-destructor-override + logan-5 wrote: > dblaikie wrote: >

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 added a comment. Feels like a dumb question, but I'm not sure if and how those build failures are related to this patch? They seem to involve completely separate parts of the LLVM project (and `.c` files to boot). Was it that the build was failing for an unrelated reason at the moment t

[PATCH] D83611: [clang][NFC] Add 'override' keyword to virtual function overrides

logan-5 created this revision. logan-5 added a project: clang. Herald added subscribers: cfe-commits, martong. This patch adds `override` to several overriding virtual functions that were missing the keyword within the clang/ directory. NFC. These were found by a Clang build equipped with `-Wsug

[PATCH] D83616: [clang] Add 'override' to virtual function overrides generated by ClangAttrEmitter

logan-5 created this revision. logan-5 added reviewers: rsmith, aaron.ballman, john.brawn. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. ClangAttrEmitter.cpp generates `ParsedAttr` derived classes with virtual overrides in them (which end up in AttrParsedAttrImpl.inc); t

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 added a comment. In D82728#2146067 , @dblaikie wrote: > Looks good - thanks for the patch and all the details! Might be worth turning > on by default in the LLVM build (after all the cleanup) Thanks a lot! I don't (think I) have commit access ya

[PATCH] D83616: [clang] Add 'override' to virtual function overrides generated by ClangAttrEmitter

logan-5 added a comment. Thanks! I would appreciate any help committing this. I have a few of these kinds of NFC `override` patches in flight, and I'd love to be able to commit them myself and not bother people. I wonder what the process of gaining commit access looks like? I've been a contribu

[PATCH] D83616: [clang] Add 'override' to virtual function overrides generated by ClangAttrEmitter

This revision was automatically updated to reflect the committed changes. Closed by commit rGfbb30c31fefc: [clang] Add 'override' to virtual function overrides generated by… (authored by logan-5). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83616/n

[PATCH] D83611: [clang][NFC] Add 'override' keyword to virtual function overrides

logan-5 closed this revision. logan-5 added a comment. Committed as rG2c2a297bb6d1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83611/new/ https://reviews.llvm.org/D83611 __

[PATCH] D83611: [clang][NFC] Add 'override' keyword to virtual function overrides

logan-5 added a comment. In D83611#2151278 , @dblaikie wrote: > If you add/leave the "Differential Revision: https://reviews.llvm.org/D"; > line in the commit message (arc will add this line automatically) Phabricator > will close the review for you

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 created this revision. logan-5 added a reviewer: rsmith. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. This patch adds `-Wsuggest-override`, which allows for more aggressive enforcement of modern C++ best practices, as well as better compatibility with gcc, whic

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 updated this revision to Diff 273986. logan-5 added a comment. Ran clang-format over the diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82728/new/ https://reviews.llvm.org/D82728 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D82728: [clang] Add -Wsuggest-override

logan-5 updated this revision to Diff 273991. logan-5 added a comment. Don't warn for destructors by default. This makes -Wsuggest-[destructor-]override more consistent with the behavior of -Winconsistent-missing-[destructor-]override, as well as gcc. Repository: rG LLVM Github Monorepo CHA

[PATCH] D74238: [clang] Improve diagnostic note for implicit conversion sequences that would work if more than one implicit user-defined conversion were allowed.

logan-5 abandoned this revision. logan-5 added a comment. @rsmith the intention is to only speculatively search one level deep (e.g. search for chains like A->B, B->C, but no deeper), which should cover most of the cases that are surprising to users. However, the implementation of this patch ha

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

logan-5 created this revision. logan-5 added a reviewer: rsmith. logan-5 added a project: clang. Herald added subscribers: cfe-commits, Prazek. A class with a destructor marked `final` cannot be derived from, so it should afford the same devirtualization opportunities as marking the entire class

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

logan-5 added a comment. That appears sort of tangential to me. To clarify, this PR is not (necessarily) about devirtualizing destructors themselves, but rather devirtualizing OTHER member function calls for classes whose destructor is marked `final`, since that is sort of morally equivalent to

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

logan-5 updated this revision to Diff 216757. logan-5 added a comment. Tweaked comment wording for accuracy. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 Files: clang/lib/AST/DeclCXX.cpp clang/test/CodeGenCXX/devir

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

logan-5 marked an inline comment as done. logan-5 added a comment. @rsmith I agree having a final destructor in a non-final class smells weird. I'd be interested in working on adding a warning like that, if it sounds like a useful thing. For now, I need help committing this, if anyone would be

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

logan-5 updated this revision to Diff 217009. logan-5 added a comment. Add a missing null check. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 Files: clang/lib/AST/DeclCXX.cpp clang/test/CodeGenCXX/devirtualize-virt

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

logan-5 created this revision. logan-5 added reviewers: rsmith, yamauchi. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. Marking a class' destructor `final` prevents the class from being inherited from. However, it is a subtle and awkward way to express that at best, and

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

logan-5 updated this revision to Diff 217049. logan-5 added a comment. Addressed some feedback. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66711/new/ https://reviews.llvm.org/D66711 Files: clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

logan-5 marked an inline comment as done. logan-5 added a comment. Thanks. I'll need someone with commit access to help me out with this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66711/new/ https://reviews.llvm.org/D66711 _

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

logan-5 added a comment. //ping// Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added a comment. In D72213#1804394 , @Mordante wrote: > I wonder what happens if the project already contains a suggested fix, for > example: > > #define __FOO(X) ... > #define _FOO(X) __FOO(X) > #define FOO(X) _FOO(X) > > > will it suggest:

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added a comment. @Eugene.Zelenko +1 to @Mordante 's question. Otherwise, happy to address most of those nits, though I think I will gently push back on a couple of them (in the inline comments). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp:21 +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 'Helper', which is not a reserved identifier [bugprone-reserved-identifier]

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 marked an inline comment as done. logan-5 added a comment. @Eugene.Zelenko never mind about pushing back on the nits; I had misread some of them. They all sound good, will address. Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:70 +

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 updated this revision to Diff 236210. logan-5 marked 39 inline comments as done. logan-5 added a comment. Addressed formatting issues, and tweaked handling of the names `__` and `::_`: the check now flags these but doesn't suggest a fix. CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:59 + struct FailureInfo { +std::string KindName; // Tag or misc info to be used as derived classes need +std::string Fixup;

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 updated this revision to Diff 236216. logan-5 added a comment. Change double-underscore fix-it finding algorithm to correctly collapse any number of >=2 underscores in a row, not just exactly 2 (or multiples of 2). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72213/new/ https:

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 created this revision. logan-5 added reviewers: aaron.ballman, hokein, alexfh. logan-5 added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. This patch adds `bugprone-unintended-adl` which flags uses of ADL that are not

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 created this revision. logan-5 added reviewers: alexfh, hokein, aaron.ballman. logan-5 added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. Before this patch, `readability-identifier-naming` contained a significant am

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added a comment. I'm taking @JonasToth's suggestion and splitting this into two patches, one for the refactor, followed by one for the new check. The refactor patch is here: https://reviews.llvm.org/D72284. Thanks everyone for your feedback so far. CHANGES SINCE LAST ACTION https://r

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 updated this revision to Diff 236420. logan-5 marked 5 inline comments as done. logan-5 added a comment. Addressed some formatting stuff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72282/new/ https://reviews.llvm.org/D72282 Files: cla

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:35 + + If non-zero, ignores calls to overloaded operators using the operator syntax (e.g. `a + b`), but not the function call syntax (e.g. `operator+(a, b)`). Defa

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 updated this revision to Diff 236449. logan-5 marked 3 inline comments as done. logan-5 added a comment. Addressed some nits. If the rest looks good, I'll need someone with commit access to help me wrap this up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:43 + Whitelist( + utils::options::parseStringList(Options.get("Whitelist", "swap"))) {} + JonasToth wro

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:272 + if (const auto *Typedef = + Value->getReturnType().getTypePtr()->getAs()) { +addUsage(NamingCheckFailures, Typedef->getDecl(),

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 updated this revision to Diff 236478. logan-5 marked 3 inline comments as done. logan-5 added a comment. More nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72284/new/ https://reviews.llvm.org/D72284 Files: clang-tools-extra/clang-t

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 updated this revision to Diff 236483. logan-5 marked 3 inline comments as done. logan-5 added a comment. Consistent-ified single-statement-body control flow statements to not use braces. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72284/n

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 updated this revision to Diff 236593. logan-5 added a comment. Clean up some false positives in macro expansions and in expressions with nested potential ADL usages (e.g. in lambdas passed as function parameters). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 marked 2 inline comments as done. logan-5 added a comment. @JonasToth Thanks for the feedback. Will be updating the diff in the next day or so. Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:89 + +const auto *Lookup = +Result.Nodes

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 added a comment. (Just in case this got lost in the shuffle) If this looks good, (I think) I'll need someone with commit access to help wrap it up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72284/new/ https://reviews.llvm.org/D72284

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 created this revision. logan-5 added reviewers: JonasToth, alexfh, aaron.ballman. logan-5 added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. This patch adds `bugprone-reserved-identifier`, which flags uses of `__name

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 updated this revision to Diff 236754. logan-5 marked an inline comment as done. logan-5 added a comment. Addressed some nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72378/new/ https://reviews.llvm.org/D72378 Files: clang-tools-ext

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 added a comment. Hi @Quuxplusone, glad you found your way here. I thought of adding you as a reviewer out the gate but then I didn't. Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:43 + Whitelist( + utils::options::parseStringLis

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 updated this revision to Diff 236884. logan-5 marked 15 inline comments as done. logan-5 added a comment. Beefed up tests and split into separate files. Added tests for instantiations of template functions that //do// definitely result in ADL, and made the warning messages more descripti

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 updated this revision to Diff 236891. logan-5 marked 3 inline comments as done. logan-5 added a comment. Added tests for template parameters. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72378/new/ https://reviews.llvm.org/D72378 Files:

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp:61 +void templateFunction(T t) { + swap(t, t); + Quuxplusone wrote: > logan-5 wrote: > > Quuxplusone wrot

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 marked 2 inline comments as done. logan-5 added a comment. In D72378#1810763 , @aaron.ballman wrote: > This check is missing a whole lot of reserved identifiers. For instance, in > C++ it is missing everything from http://eel.is/c++draft/zombie.n

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 updated this revision to Diff 237244. logan-5 marked 8 inline comments as done. logan-5 added a comment. Addressed nits. Added CERT aliases. Adjusted the check to work for both C and C++, including where they differ. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 updated this revision to Diff 237433. logan-5 marked 4 inline comments as done. logan-5 added a comment. Added a TODO comment for catching more reserved names. Added links in documentation to CERT guidelines covered by the check. Pulled strings into named constants and made that logic ea

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 updated this revision to Diff 238014. logan-5 added a comment. Rebased with trunk. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72284/new/ https://reviews.llvm.org/D72284 Files: clang-tools-extra/clang-tidy/readability/IdentifierNamingC

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:29 +public: + RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context); + ~RenamerClangTidyCheck(); njam

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 updated this revision to Diff 238064. logan-5 marked 6 inline comments as done. logan-5 added a comment. Renamed check option from "Whitelist" to "AllowedIdentifiers". Added note about missing checks in documentation. Changed to use a %select for diagnostic text. Some nits. The check do

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

logan-5 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:27-34 +static const char NonReservedMessage[] = +"declaration uses identifier '%0', which is not a reserved " +"identifier"; +static const char GlobalUnderscoreMe

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

logan-5 marked 2 inline comments as done. logan-5 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:29 +public: + RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context); + ~RenamerClangTidyCheck(); loga

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 updated this revision to Diff 238328. logan-5 added a comment. Added TODO comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72282/new/ https://reviews.llvm.org/D72282 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.c

[PATCH] D102338: [Sema] Always search the full function scope context if a potential availability violation is encountered

logan-5 created this revision. logan-5 added reviewers: erik.pilkington, rsmith, arphaman. logan-5 added a project: clang. logan-5 requested review of this revision. Herald added a subscriber: cfe-commits. Always search the full function scope context if a potential availability violation is enco

[PATCH] D102338: [Sema] Always search the full function scope context if a potential availability violation is encountered

logan-5 added a comment. Thanks very much; I look forward to any feedback! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102338/new/ https://reviews.llvm.org/D102338 ___ cfe-commits mailing list cfe-comm

[PATCH] D102338: [Sema] Always search the full function scope context if a potential availability violation is encountered

This revision was automatically updated to reflect the committed changes. Closed by commit rGa5a3efa82a77: [Sema] Always search the full function scope context if a potential… (authored by logan-5). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D10233

[PATCH] D74238: [clang] Improve diagnostic note for implicit conversion sequences that would work if more than one implicit user-defined conversion were allowed.

logan-5 added a comment. Pinging this and the patches it depends on. I figured it would need a rebase by now, but it still applies cleanly to trunk. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74238/new/ https://reviews.llvm.org/D74238 __

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 added a comment. Just giving this a friendly ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72282/new/ https://reviews.llvm.org/D72282 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 marked 2 inline comments as done. logan-5 added a comment. Thanks for the feedback. In D72282#1918253 , @EricWF wrote: > - This check should suggest fixes. It knows what function is actually being > resolved, and it has enough information to crea

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp:202 +void macro_test(T t) { +#define MACRO(x) find_if(x, x, x) + Quuxplusone wrote: > logan-5 wrote: > > E

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 updated this revision to Diff 250130. logan-5 marked 3 inline comments as done. logan-5 added a comment. The check now suggests fixes for concrete (i.e. non-template) uses of ADL. Re-enabled the check for macros, and eliminated some false positives with arrays of dependent size. The chec

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 updated this revision to Diff 250272. logan-5 marked 2 inline comments as done. logan-5 added a comment. Re-juggled and made `bool OnlyDependsOnFundamentalArraySizes(QualType QualTy)` recursive. Made `::` assert more useful. Thanks @Quuxplusone for both good ideas. CHANGES SINCE LAST A

[PATCH] D74009: [clang] Improve diagnostic note for implicit conversions that are disallowed because they involve more than one user-defined conversion.

logan-5 created this revision. logan-5 added a reviewer: rsmith. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. The current text of the 'note' diagnostic for bad conversions is confusing in the presence of user-defined conversion operations: https://godbolt.org/z/zrgeHH F

[PATCH] D74009: [clang] Improve diagnostic note for implicit conversions that are disallowed because they involve more than one user-defined conversion.

logan-5 added a comment. In D74009#1858353 , @rsmith wrote: > Have you looked into whether you can defer the check for conversion via > multiple user-defined conversions until we come to call > `CompleteNonViableCandidate` after finding no viable functio

[PATCH] D74234: [clang] [NFC] Change boolean SuppressUserConversions parameters to an enum

logan-5 created this revision. logan-5 added a reviewer: rsmith. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. logan-5 added a child revision: D74238: [clang] [clang] Improve diagnostic note for implicit conversion sequences that would work if more than one implicit user

[PATCH] D74238: [clang] [clang] Improve diagnostic note for implicit conversion sequences that would work if more than one implicit user-defined conversion were allowed.

logan-5 created this revision. logan-5 added a reviewer: rsmith. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. The current text of the 'note' diagnostic for bad conversions is confusing in the presence of user-defined conversion operations: https://godbolt.org/z/zrgeHH F

[PATCH] D74009: [clang] Improve diagnostic note for implicit conversions that are disallowed because they involve more than one user-defined conversion.

logan-5 abandoned this revision. logan-5 added a comment. I split this patch into multiple, and reworked the implementation. The new stuff lives here and here and here: D74238 D74234 D74235 R

[PATCH] D74235: [clang] [NFC] Rename Sema::DiagnoseMultipleUserConversion to DiagnoseAmbiguousUserConversion

logan-5 created this revision. logan-5 added a reviewer: rsmith. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. logan-5 added a child revision: D74238: [clang] [clang] Improve diagnostic note for implicit conversion sequences that would work if more than one implicit user

  1   2   >