[PATCH] D152764: [clang-tidy] Reserved-identifier: Improved AllowedIdentifiers option to support regular expressions

logan-5 added a comment. Thanks for the patch. This seems like it'll make the check much more useful. Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:48 + Options.get("AllowedIdentifiers", ""))) { + for (const auto &Identifier : AllowedId

[PATCH] D116824: [clang-tidy] Fix RenamerClangTidyChecks suggesting invalid macro identifiers

This revision was automatically updated to reflect the committed changes. Closed by commit rG988c3f5f9692: [clang-tidy] Fix RenamerClangTidyChecks suggesting invalid macro identifiers (authored by logan-5). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D116824: [clang-tidy] Fix RenamerClangTidyChecks suggesting invalid macro identifiers

logan-5 created this revision. logan-5 added reviewers: njames93, aaron.ballman. logan-5 added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun. logan-5 requested review of this revision. Herald added a subscriber: cfe-commits. This behavior was fixed for regular id

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

This revision was automatically updated to reflect the committed changes. Closed by commit rG715c72b4fbf4: [NFC][analyzer] Return underlying strings directly instead of OS.str() (authored by logan-5). Changed prior to commit: https://reviews.llvm.org/D115374?vs=393262&id=393319#toc Repository:

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

logan-5 updated this revision to Diff 393262. logan-5 added a comment. Removed `.flush()`es. Seems like this might be able to land without https://reviews.llvm.org/D115421? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115374/new/ https://reviews.

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

logan-5 added a comment. To be clear, it sounds like we should //either// add `.take()` for moving the string out of `raw_string_ostream`'s string reference, //or// make `raw_string_ostream` not need to be flushed (after which there won't be as clear a use for `.take()`, since you can just use

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

logan-5 updated this revision to Diff 392996. logan-5 added a comment. Eliminate some explicit `.flush()`es by using temporary `raw_string_ostream`s where possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115374/new/ https://reviews.llvm.org

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

logan-5 added a comment. In D115374#3181383 , @dexonsmith wrote: > I don't feel strongly, but IMO the code might be a bit harder to > read/maintain with the explicit flush. I worry that it'd be easy to move the > `flush()` away from the `return`. Not s

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

logan-5 added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36 +OS.flush(); +return Str; } Quuxplusone wrote: > FWIW, it appears to me that in most (all?) of these cases, what's really > wanted is not "a stri

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

logan-5 created this revision. logan-5 added a reviewer: aaron.ballman. logan-5 added a project: clang. Herald added subscribers: abrachet, ctetreau, dexonsmith, martong. logan-5 requested review of this revision. Herald added a subscriber: cfe-commits. Returning `OS.str()` is guaranteed to copy t

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

logan-5 added a comment. It's been a year and a half since I heard anything about this patch and I'd honestly kind of forgotten about it. I'm still around/lurking though, and would assist with getting it landed if it's still good to go. Comment at: clang-tools-extra/clang-tid

[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] 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

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] 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] 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

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] 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] 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#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. 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#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. 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#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] 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 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/

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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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 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 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 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 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 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 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 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 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 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 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] 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 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] 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] 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] 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 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 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 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] 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 updated this revision to Diff 243307. logan-5 marked an inline comment as done. logan-5 added a comment. Changed `Whitelist` to `AllowedIdentifiers`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72282/new/ https://reviews.llvm.org/D72282

[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

[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] 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] 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] D72282: [clang-tidy] Add `bugprone-unintended-adl`

logan-5 updated this revision to Diff 240691. logan-5 added a comment. Rebased with trunk. Updated whitelist to include more standard designated customization points . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

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

logan-5 updated this revision to Diff 238640. logan-5 added a comment. Should be good now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72378/new/ https://reviews.llvm.org/D72378 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.c

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

logan-5 added a comment. Great! In that case, I'll need help committing this, as well as the thing it depends on, https://reviews.llvm.org/D72284 (which has also been LGTM'd). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72378/new/ https://review

[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] 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] 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] 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] 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] 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] 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] 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] 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 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 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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 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 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] 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] 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] 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] 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] 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 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 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 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]

  1   2   >