[PATCH] D24888: [clang-tidy] Use [[clang::suppress]] with cppcoreguidelines-pro-type-reinterpret-cast

mgehre added a comment. In D24888#4025382 , @bam wrote: > I'm sorry, could we bring it to the finish line? > It's a pity the standard C++ Core Guidelines rule suppression syntax is still > not supported, as Clang-tidy is one of the main tools supporting t

[PATCH] D139170: [X86][clang] Lift _BitInt() supported max width.

mgehre-amd added a comment. In D139170#3965850 , @FreddyYe wrote: > In D139170#3965814 , @mgehre-amd > wrote: > >> Do other targets not support > 128 bit integers, or is this PR only the >> first conservative st

[PATCH] D139170: [X86][clang] Lift _BitInt() supported max width.

mgehre-amd added a comment. Do other targets not support > 128 bit integers, or is this PR only the first conservative step of lifting the limit? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139170/new/ https://reviews.llvm.org/D139170 _

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt

mgehre-amd abandoned this revision. mgehre-amd added a comment. Instead of linking to libbitint, I instead created a backend pass to transform div/rem instructions: https://reviews.llvm.org/D130076 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1222

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

mgehre-amd closed this revision. mgehre-amd added a comment. Merged Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127287/new/ https://reviews.llvm.org/D127287 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

mgehre-amd added a comment. Thanks for the quick review! Comment at: clang/test/Sema/large-bit-int.c:1 +// RUN: %clang_cc1 -fexperimental-max-bitint-width=1024 -fsyntax-only -verify %s -Wno-unused -triple x86_64-gnu-linux + aaron.ballman wrote: > Thoughts on a

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

mgehre-amd updated this revision to Diff 435161. mgehre-amd marked 4 inline comments as done. mgehre-amd added a comment. Used Optional Added entry to release notes Modifed help text according to suggestion Added comment to LANGOPT that it will be removed in the future Repository: rG LLVM Gith

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt

mgehre-amd marked an inline comment as done. mgehre-amd added a comment. I split the introduction of -fexperimental-max-bitint-width into https://reviews.llvm.org/D127287 because there is still discussion about the bitint libraries in the backend PRs. Repository: rG LLVM Github Monorepo CHA

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

mgehre-amd created this revision. mgehre-amd added a reviewer: aaron.ballman. Herald added a project: All. mgehre-amd requested review of this revision. Herald added a project: clang. This splits of the introduction of -fexperimental-max-bitint-width from https://reviews.llvm.org/D122234 because

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

mgehre-amd added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:315 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description) -#define BENIGN_VALUE_LANGOPT(Name, Type, Bits, Default, Description) +#define BENIGN_VALUE_LANGOPT(Name, Bits, Default, De

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

mgehre-amd updated this revision to Diff 418861. mgehre-amd added a comment. Herald added a subscriber: dexonsmith. - Remove diagnostics for float-to-bitint - Remove lexer changes (will be another PR) - Add -fexperimental-max-bitint-width=N Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

mgehre-amd marked 2 inline comments as done. mgehre-amd added inline comments. Herald added a subscriber: MaskRay. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8422 +def err_int_to_float_bit_int_max_size : Error< + "cannot convert '_BitInt' operands of more tha

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

mgehre-amd marked 3 inline comments as done. mgehre-amd added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8422 +def err_int_to_float_bit_int_max_size : Error< + "cannot convert '_BitInt' operands of more than %0 bits to floating point">; -

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

mgehre-amd marked an inline comment as done. mgehre-amd added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8422 +def err_int_to_float_bit_int_max_size : Error< + "cannot convert '_BitInt' operands of more than %0 bits to floating point">; -

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

mgehre-amd created this revision. mgehre-amd added reviewers: aaron.ballman, erichkeane. Herald added subscribers: luke957, s.egerton, mstorsjo, simoncook, fedor.sergeev, dschuff. Herald added a project: All. mgehre-amd requested review of this revision. Herald added subscribers: pcwang-thead, ahe

[PATCH] D122227: Fix _BitInt suffix width calculation

mgehre-amd accepted this revision. mgehre-amd added a comment. Thanks a lot! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D17/new/ https://reviews.llvm.org/D17 ___ cfe-commits mailing list cfe-co

[PATCH] D122075: [clang-tidy] Skip parentheses in `readability-make-member-function-const`

mgehre-amd added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122075/new/ https://reviews.llvm.org/D122075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre added a comment. Sorry for breaking the build and thanks for the fixes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm.org/D77572 ___ cfe-commits mailing list cfe-

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

This revision was automatically updated to reflect the committed changes. Closed by commit rGadd51e152aa6: [clang-tidy] add new check readability-use-anyofallof (authored by mgehre). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://r

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre updated this revision to Diff 268120. mgehre added a comment. Implemented njames93's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm.org/D77572 Files: clang-tools-extra/clang-tidy/readability/CMakeLi

[PATCH] D80301: [yaml][clang-tidy] Fix new line YAML serialization

mgehre added inline comments. Comment at: llvm/lib/Support/YAMLTraits.cpp:904 + std::string &Val) { + Val.clear(); + size_t CurrentPos = 0; DmitryPolukhin wrote: > mgehre wrote: > > I wonder whether using StringRef::spl

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre updated this revision to Diff 266083. mgehre marked an inline comment as done. mgehre added a comment. Fix comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm.org/D77572 Files: clang-tools-extra/clang-ti

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre marked 5 inline comments as done. mgehre added a comment. Aaron, thank you very much for taking the time to review this PR. Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:61 + hasBody(allOf(hasDescendant(returns(true)), +

[PATCH] D80301: [yaml][clang-tidy] Fix new line YAML serialization

mgehre added a comment. Thanks for doing this! I didn't work on the YAML parser before, so I cannot give formal approval. Comment at: llvm/lib/Support/YAMLTraits.cpp:904 + std::string &Val) { + Val.clear(); + size_t CurrentPos = 0; -

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre added a comment. Ping :-) I'm looking for directions on what the next steps are. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm.org/D77572 ___ cfe-commits mailing

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

mgehre added a comment. Could be - did it handle it correctly for your include case here? And if this is a general yaml string thing, shouldn't the replacement of newlines (for both ways) happen in the yaml parser/writer instead of clang/Tooling? Repository: rL LLVM CHANGES SINCE LAST ACTI

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

mgehre added a comment. This change breaks `modernize-use-using` fixits. The code `typedef int A, B;` is replaced into using A = int; using B = int; when directly applying fixits with clang-tidy. But it is replaced into using A = int; using B = int; when applying fixits with `clang-

[PATCH] D77979: [clang-tidy] modernize-use-using: Fix broken fixit with InjectedClassName

This revision was automatically updated to reflect the committed changes. Closed by commit rG145dcef8bdf9: [clang-tidy] modernize-use-using: Fix broken fixit with InjectedClassName (authored by mgehre). Changed prior to commit: https://reviews.llvm.org/D77979?vs=256872&id=260294#toc Repository

[PATCH] D78139: [clang-tidy] modernize-use-using: Fix broken fixit with 'template' keyword

This revision was automatically updated to reflect the committed changes. Closed by commit rG0642e5e7a7e5: [clang-tidy] modernize-use-using: Fix broken fixit with 'template' keyword (authored by mgehre). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

mgehre added inline comments. Comment at: test/clang-tidy/bugprone-map-subscript-operator-lookup.cpp:60 +} + +void noWarning() { We often have code like ``` auto &V = Map[Idx]; if (!V) { V = init(); } use(V); ``` Would that be flagged by this check? Could you a

[PATCH] D78139: [clang-tidy] modernize-use-using: Fix broken fixit with 'template' keyword

mgehre updated this revision to Diff 257774. mgehre marked an inline comment as done. mgehre added a comment. Implement review comments (Thanks!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78139/new/ https://reviews.llvm.org/D78139 Files: cla

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre added a comment. Thanks for the comments so far. I'm a bit lost now. Which changes that cannot wait for the next PR do you see necessary to get this check merged? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm

[PATCH] D78139: [clang-tidy] modernize-use-using: Fix broken fixit with 'template' keyword

mgehre created this revision. mgehre added reviewers: aaron.ballman, alexfh, hokein, njames93. Herald added a subscriber: xazax.hun. Herald added a project: clang. Before this PR, `modernize-use-using` would transform the typedef in template class TemplateKeyword { typedef typename a::temp

[PATCH] D77979: [clang-tidy] modernize-use-using: Fix broken fixit with InjectedClassName

mgehre created this revision. mgehre added reviewers: aaron.ballman, alexfh, hokein, njames93. Herald added a subscriber: xazax.hun. Herald added a project: clang. Before this PR, `modernize-use-using` would transform the typedef in template struct InjectedClassName { typedef InjectedCla

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre added a comment. Thanks @njames93, this is a good examples which I did not consider yet. I agree that only transforming the second loop would make the code worse. I would argue that a human seeing a warning on the second loop might also realize that the first loop can be transformed in a

[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

This revision was automatically updated to reflect the committed changes. Closed by commit rGeaa55590945a: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda (authored by mgehre). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D776

[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

mgehre added a comment. In D77680#1968860 , @njames93 wrote: > Outright disabling for lambdas probably isn't the ideal solution, however > it's at least a good stepping stone til a better solution can be found. I fully agree! Repository: rG LLVM Git

[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

mgehre created this revision. mgehre added reviewers: aaron.ballman, alexfh, hokein, njames93. Herald added a subscriber: xazax.hun. Herald added a project: clang. mgehre added a project: clang-tools-extra. mgehre edited the summary of this revision. Previously, the check would fix using fn = v

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre updated this revision to Diff 255814. mgehre added a comment. Review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm.org/D77572 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre updated this revision to Diff 255812. mgehre added a comment. Review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm.org/D77572 Files: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre marked 6 inline comments as done. mgehre added a comment. In D77572#1965143 , @njames93 wrote: > I'm struggling to see the value of this check though. If it was reworked to > check for loop in the middle of a function it would have a lot more value

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

mgehre created this revision. mgehre added reviewers: aaron.ballman, alexfh, hokein. Herald added subscribers: xazax.hun, mgorny. Herald added a project: clang. Finds range-based for loops that can be replaced by a call to ``std::any_of`` or ``std::all_of``. In C++ 20 mode, suggests ``std::ranges:

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

mgehre added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h:20 +namespace clang { +template class DataflowWorklistBase { + llvm::BitVector EnqueuedBlocks; xazax.hun wrote: > xazax.hun wrote: > > mgehre wrote: > > > Sh

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

mgehre added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h:20 +namespace clang { +template class DataflowWorklistBase { + llvm::BitVector EnqueuedBlocks; Should this class have a bit of doxygen and a unit test? CHA

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

mgehre added a comment. Did you consider to warn on copy constructors/assignment operators take a their arguments by non-`const` reference, and suggest the user to turn them into const references? This seems like a more useful (and easier) check to me. The link above contains `Ideally, the copy

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

mgehre added a comment. Have you run your check over the LLVM/clang source base and seen true positives/false positives? Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-mutating-copy.rst:6 + +Finds assignments to and to direct or indirect members of the copied object

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

This revision was automatically updated to reflect the committed changes. Closed by commit rG24130d661ed4: [clang-tidy] Add readability-make-member-function-const (authored by mgehre). Changed prior to commit: https://reviews.llvm.org/D68074?vs=225804&id=228006#toc Repository: rG LLVM Github

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre marked 8 inline comments as done. mgehre added a comment. Thanks, fixed! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68074/new/ https://reviews.llvm.org/D68074 ___ cfe-commits mailing list cfe

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre added a comment. Friendly Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68074/new/ https://reviews.llvm.org/D68074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre updated this revision to Diff 225804. mgehre marked 6 inline comments as done. mgehre added a comment. - Update documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68074/new/ https://reviews.llvm.org/D68074 Files: clang-tools-extr

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

mgehre added a comment. Exactly due to the issue you are fixing here, we ended up disabling the complete check because we didn't want to live with the warnings it produced on -Wextra. Therefore, I'm actually strongly in favor to enable the option by default. When others see that clang-tidy fixi

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

mgehre added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp:55 const auto *Construct = Result.Nodes.getNodeAs("construct"); + const auto* ConstructorDecl = Result.Nodes.getNodeAs( "constructor" ); + Extra

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre marked 2 inline comments as done. mgehre added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst:10 +The check conservatively tries to preserve logical costness in favor of +physical costness. The only operati

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre marked an inline comment as done. mgehre added inline comments. Comment at: clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp:311 + // This member function must stay non-const, even though + // it only calls other private const member function

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre added a comment. In D68074#1683770 , @lebedev.ri wrote: > Awesome! > I believe i have asked this question in the convert-to-static diff - can > ExprMutAnalyzer be used here? I believe in this case, we can get away with a simpler analysis. Becaus

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre updated this revision to Diff 222331. mgehre marked 6 inline comments as done. mgehre added a comment. - Implement review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68074/new/ https://reviews.llvm.org/D68074 Files: clang-tools

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

mgehre created this revision. mgehre added reviewers: aaron.ballman, gribozavr, hokein, alexfh. mgehre added a project: clang. Herald added subscribers: xazax.hun, mgorny. Finds non-static member functions that can be made ``const`` because the functions don't use ``this`` in a non-const way. The

[PATCH] D66365: [clang-tidy] [readability-convert-member-functions-to-static] ignore functions that hide base class methods

mgehre abandoned this revision. mgehre added a comment. The discussions on the bug did not provide further insight (until now). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66365/new/ https://reviews.llvm.org/D66365

[PATCH] D66806: [LifetimeAnalysis] Fix some false positives

mgehre added a comment. Nice! Having no false-positives is most important because this is enabled by default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66806/new/ https://reviews.llvm.org/D66806 _

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

mgehre added a comment. I'm seeing the same issue `Not all CodeGen sections are inside any Frontend section!` with python 3.7.1. Json: F9863382: check-time-trace-sections.json Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325

[PATCH] D66686: [LifetimeAnalysis] Make it possible to disable the new warnings

mgehre added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6651 visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, -Visit); +Visit, true); else --

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

mgehre added a comment. Yes, it means that the automatic inference of Pointer/Owner from base classes has the same issues as all of those automatic inferences: Once can construct a case where they are not true. So for having no false-positives at all, we should avoid this inference by default a

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

mgehre added a comment. When I understand you correctly, you are thinking about the cases OwningArrayRef getRef(); ArrayRef f() { ArrayRef r = getRef(); // warning: r points into temporary owner return r; } and ArrayRef getRef() { OwningArrayRef r; return r; // warni

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

mgehre added a comment. > Also it feels a bit weird to change the ownership semantics in a derived > class, I bet that would violate the Liskov substitution principle. And then we see that llvm::OwningArrayRef inherits from llvm::ArrayRef ... But maybe this direction (strengthening a Pointer in

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre marked an inline comment as done. mgehre added a comment. Yes, for libc++ we could add the annotations to its source code, and eventually this would be the cleaner solution. Currently, that is neither sufficient (because one could use libstdc++, MSVC or an older libc++ version) nor neces

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre marked 2 inline comments as done. mgehre added inline comments. Comment at: cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp:9 + +#include "../ASTMatchers/ASTMatchersTest.h" +#include "clang/ASTMatchers/ASTMatchers.h" thakis wrote: > mgehre wrote: > >

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre marked an inline comment as done. mgehre added inline comments. Comment at: cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp:9 + +#include "../ASTMatchers/ASTMatchersTest.h" +#include "clang/ASTMatchers/ASTMatchers.h" thakis wrote: > This weird relati

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

mgehre added a comment. In the false-positive example, after the `DerivedToBase`, we see a constructor call which I think is the copy constructor. 1. We should consider `MutableArrayRef` to be a gsl::Pointer according to the paper, because it publicly derives from one. 2. Also in the paper, the

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

This revision was automatically updated to reflect the committed changes. mgehre marked an inline comment as done. Closed by commit rL369591: [LifetimeAnalysis] Support more STL idioms (template forward declaration and… (authored by mgehre, committed by ). Herald added a project: LLVM. Herald adde

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre marked 3 inline comments as done. mgehre added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4596 +for (Decl *Redecl : D->redecls()) { + Redecl->addAttr(::new (S.Context) + OwnerAttr(AL.getRange(), S.Context, DerefTypeLoc, -

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

mgehre added a comment. This change might be the cause for the false-positive in https://github.com/mgehre/llvm-project/issues/45. Could you check? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66486/new/ https://reviews.llvm.org/D66486 ___

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre updated this revision to Diff 216247. mgehre marked 6 inline comments as done. mgehre added a comment. Herald added a subscriber: mgorny. - Put OwnerAttr/PointerAttr on all redeclarations - clang/lib/Sema/SemaAttr.cpp: Use Attribute::CreateImplicit as requested in review Repository: rG

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre added a comment. I now add the attributes to all redeclarations to make the logic a bit easier to follow. Comment at: clang/lib/Sema/SemaAttr.cpp:94 - Canonical->addAttr(::new (Context) Attribute(SourceRange{}, Context, -

[PATCH] D66365: [clang-tidy] [readability-convert-member-functions-to-static] ignore functions that hide base class methods

mgehre added a comment. I struggle myself to fully understand the motivation behind the bug report. This is certainly not code that I would write. So I don't really care about that kind of code and I didn't want to be in the way of other people's way of writing C++. Alternatively, we could ask

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre marked 4 inline comments as done. mgehre added a comment. I now start to wonder if we should instead put the attribute on all declarations (instead of just the canonical). Later redeclarations automatically inherit the attributes. This would make both the checking easier (just check any d

[PATCH] D66365: [clang-tidy] [readability-convert-member-functions-to-static] ignore functions that hide base class methods

mgehre created this revision. mgehre added reviewers: aaron.ballman, Eugene.Zelenko. Herald added a subscriber: xazax.hun. Herald added a project: clang. Fixes bug https://bugs.llvm.org/show_bug.cgi?id=43002 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66365 Files: clang-t

[PATCH] D66303: [LifetimeAnalysis] Add support for free functions

mgehre added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6622 +return false; + const auto *RD = FD->getParamDecl(0)->getType()->getPointeeCXXRecordDecl(); + if (!FD->isInStdNamespace() || !RD || !RD->isInStdNamespace()) Maybe move the `Callee->

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre marked 6 inline comments as done. mgehre added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:102 + dyn_cast_or_null(Record->getDescribedTemplate())) { +if (auto *Def = Record->getDefinition()) + addGslOwnerPointerAttributeIfNotExisting(Context

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

This revision was automatically updated to reflect the committed changes. Closed by commit rL368929: [LifetimeAnalysis] Support std::stack::top() and std::optional::value() (authored by mgehre, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

mgehre updated this revision to Diff 214954. mgehre marked an inline comment as done. mgehre added a comment. Add tests for rvalue-ref overloads Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66164/new/ https://reviews.llvm.org/D66164 Files: clan

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

mgehre marked 3 inline comments as done. mgehre added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6583 .Cases("end", "rend", "cend", "crend", true) -.Cases("c_str", "data", "get", true) +.Cases("c_str", "data", "get", "value", true)

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

mgehre created this revision. mgehre added a reviewer: gribozavr. Herald added a subscriber: Szelethus. Herald added a project: clang. This fixes inference of gsl::Pointer on std::set::iterator with libstdc++ (the typedef for iterator on the template is a DependentNameType - we can only put the g

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

mgehre updated this revision to Diff 214950. mgehre marked 2 inline comments as done. mgehre added a comment. Fix commit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66164/new/ https://reviews.llvm.org/D66164 Files: clang/lib/Sema/SemaInit.cpp

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

mgehre marked an inline comment as done. mgehre added inline comments. Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:172 T &operator*(); + T &value(); +}; xazax.hun wrote: > I actually was a bit lazy when I added these tests. Both `value` and

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

mgehre created this revision. mgehre added a reviewer: gribozavr. Herald added a project: clang. Diagnose dangling pointers that come from std::stack::top() and std::optional::value(). Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66164 Files: clang/lib/Sema/SemaInit.cpp

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

This revision was automatically updated to reflect the committed changes. Closed by commit rL368147: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types (authored by mgehre, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

mgehre updated this revision to Diff 212446. mgehre added a comment. - Fix crash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64448/new/ https://reviews.llvm.org/D64448 Files: clang/include/clang/Basic/AttrDocs.td clang/include/clang/Sema/Sem

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

mgehre updated this revision to Diff 212441. mgehre added a comment. - Add missing check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64448/new/ https://reviews.llvm.org/D64448 Files: clang/include/clang/Basic/AttrDocs.td clang/include/clang/

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

mgehre added inline comments. Comment at: clang/include/clang/Sema/Sema.h:6097 + + /// Add [[gsl::Owner]] and [[gsl::Pointer]] attributes for std:: types. + void addDefaultGslPointerAttribute(TypedefNameDecl *TD); gribozavr wrote: > It seems like this function

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

mgehre updated this revision to Diff 212439. mgehre marked 6 inline comments as done. mgehre added a comment. - Fix comments - Add Pointer via typedef on ClassTemplateDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64448/new/ https://reviews.llv

[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. I will include your latest comments into D64448 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 ___ cfe-commits maili

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

mgehre added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6581 +if (!Callee->getIdentifier()) { + auto OO = Callee->getOverloadedOperator(); + return OO == OverloadedOperatorKind::OO_Subscript || xazax.hun wrote: > If we want to relax th

[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. Thank you very much for dedicating your time for the review. It improved the code a lot! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 ___ cfe-commit

[PATCH] D63954: Add lifetime categories attributes

This revision was automatically updated to reflect the committed changes. Closed by commit rL367040: Add lifetime categories attributes (authored by mgehre, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D63954: Add lifetime categories attributes

mgehre marked 9 inline comments as done. mgehre added inline comments. Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp:2 +// RUN: %clang_cc1 -fsyntax-only -verify %s + +int [[gsl::Owner]] i; aaron.ballman wrote: > This file tests part of parsing

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 211600. mgehre marked an inline comment as done. mgehre added a comment. - Fix all comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 Files: clang/include/clang/

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 211370. mgehre marked 4 inline comments as done. mgehre added a comment. - Diagnose unions; improve other diagnostics; fix all comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.ll

[PATCH] D63954: Add lifetime categories attributes

mgehre marked 2 inline comments as done. mgehre added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4167 + +The attribute ``[[gsl::Owner(T)]]`` applies to structs and classes that own an +object of type ``T``: aaron.ballman wrote: > Do either

[PATCH] D15032: [clang-tidy] new checker cppcoreguidelines-pro-lifetime

mgehre abandoned this revision. mgehre added a comment. Herald added subscribers: wuzish, dmgreen. Upstreaming of this checker will happen with D63954 and following revisions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15032/new/ https://reviews.llv

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

mgehre added a comment. In D64448#1595017 , @lebedev.ri wrote: > Just passing-by thought: > These attributes that are being added implicitly, it will be possible to > differentiate > whether an attribute was actually present in the code, or was added >

  1   2   >