isuckatcs added a comment.
> I'm wondering if we should have a window for backward compatibility; There
> are a couple of places already where we check for the presence of some
> metadata and display it only if it exists.
> Do you think it would be useful for this change as well?
It has been fi
isuckatcs updated this revision to Diff 456715.
isuckatcs marked an inline comment as done.
isuckatcs added a comment.
Removed the unnecessary extra RUN command.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132654/new/
https://reviews.llvm.org/D132654
Files:
clang-tools-extra/clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcd40245f549b: [clang-tidy] Fix false positive on
ArrayInitIndexExpr inside… (authored by isuckatcs).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132654/new
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb5147937b2a9: [analyzer] Add more information to the
Exploded Graph (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
C
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa46154cb1cd0: [analyzer] Warn if the size of the array in
`new[]` is undefined (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa11e51e91f73: [analyzer] Track trivial copy/move
constructors and initializer lists in the… (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d2e830737bc: [analyzer] Fix BindingDecl evaluation for
reference types (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorep
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a13326d184c: [analyzer] ArrayInitLoopExpr with array of
non-POD type (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscrib
This revision was automatically updated to reflect the committed changes.
Closed by commit rG996b092c5e17: [analyzer] Lambda capture non-POD type array
(authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://revie
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa618d5e0dd5d: [analyzer] Structured binding to tuple-like
types (authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Repository:
rG LL
This revision was automatically updated to reflect the committed changes.
Closed by commit rG10a7ee0bac21: [analyzer] Fix for the crash in #56873
(authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LA
isuckatcs added a comment.
> Some checker should have caught the uninitialized value earlier than the
> defaultEvalCall().
> I guess, the MallocCkecher could have checked for it in PreStmt.
> Or alternatively, the CallAndMessageChecker::preCall() already does something
> like this in the PreVisi
isuckatcs added a comment.
> If so, shouldn't be some dependencies across these revisions?
I don't think they are that closely related.
This patch is about fixing an assertion failure. This assertion failure happens
because we don't handle a case not because the checker doesn't exist.
Also the
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb032e3ff6121: [analyzer] Evaluate construction of non-POD
type arrays (authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Repository:
isuckatcs added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:157
"Storage provided to placement new is only {0} bytes, "
-"whereas the allocated array type requires more space for "
-"internal needs",
-
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea8aebf9eb7f: [analyzer] Move unexecuted test block into
it's own source file (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github M
isuckatcs added a comment.
> This eliminate the crash in `getDynamicElementCount` on that region
I think that if the crash comes from `getDynamicElementCount`, we should
address it there too, so
when the function is called elsewhere under the same circumstances it won't
crash again.
What do y
isuckatcs added a comment.
> For me, it makes sense that getDynamicElementCount() is not intended to be
> called on a null MemRegion.
It would still be helpful to have an assertion on that inside
`getDynamicElementCount()`, so the caller will know it right away.
What I want to say is that it s
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb4e3e3a3eb77: [analyzer] Fix a crash on copy elided
initialized lambda captures (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa47ec1b79797: [analyzer][NFC] Be more descriptive when we
replay without inlining (authored by isuckatcs).
Herald added a project: clang.
Herald adde
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c482632e64c: [analyzer] Remove pattern matching of lambda
capture initializers (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc81bf940c77e: [analyzer] Handling non-POD multidimensional
arrays in ArrayInitLoopExpr (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaac73a31add5: [analyzer] Process non-POD array element
destructors (authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe3e9082b013b: [analyzer] Fix for incorrect handling of 0
length non-POD array construction (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG
isuckatcs created this revision.
isuckatcs added reviewers: aaron.ballman, LegalizeAdulthood, njames93.
Herald added subscribers: carlosgalvezp, arphaman, kbarton, xazax.hun, nemanjai.
Herald added a project: All.
isuckatcs requested review of this revision.
Herald added a project: clang-tools-extr
isuckatcs added a comment.
@steakhal
Can you send me a snippet please, which reproduces this issue? For me the
egraph rewriter works fine.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127973/new/
https://reviews.llvm.org/D127973
___
isuckatcs added a comment.
> I had the time for reducing it. Luckily it wasn't that bad.
This egraph doesn't contain an entry with key "index_of_element", which is
strange.
I still feel like I need a code snippet, which generates such egraph.
Repository:
rG LLVM Github Monorepo
CHANGES SIN
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6931d311eaf4: [analyzer] Cleanup some artifacts from non-POD
array evaluation (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github M
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3b652fc6d644: [analyzer] Fix static code analysis concerns
(authored by Manna, committed by isuckatcs).
Herald added a project: clang.
Herald added a
isuckatcs created this revision.
isuckatcs added reviewers: njames93, baloghadamsoftware, aaron.ballman,
LegalizeAdulthood.
Herald added subscribers: carlosgalvezp, manas, ASDenysPetrov, dkrupp,
donat.nagy, Szelethus, a.sidorin, rnkovacs, xazax.hun.
Herald added a project: All.
isuckatcs requeste
isuckatcs updated this revision to Diff 466204.
isuckatcs edited the summary of this revision.
isuckatcs added a comment.
Updated
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
isuckatcs updated this revision to Diff 466269.
isuckatcs marked an inline comment as done.
isuckatcs added a comment.
Addressed inline comment
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/Excepti
isuckatcs updated this revision to Diff 466312.
isuckatcs marked 2 inline comments as done.
isuckatcs added a comment.
Addressed comments and added support for multi-level pointers too.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
isuckatcs added a comment.
I've found a strange scenario.
The following conversions are allowed
double *a[2][3];
double const * const (*ap)[3] = a; // OK
double * const (*ap1)[] = a; // OK since C++20
However if the same conversion is supposed to be performed in a `catch()`
stateme
isuckatcs marked an inline comment as done.
isuckatcs added inline comments.
Comment at: clang/test/Analysis/lambdas.cpp:226
+ [uniquePtr = MakeUniquePtr()] {}();
+ clang_analyzer_warnIfReached(); // expected-warning{{TRUE}}
+}
steakhal wrote:
> It should have
isuckatcs added a comment.
ping
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
isuckatcs added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1244
SVal *ElementCountVal) {
+ assert(Region != nullptr && "Null-regions passed to
prepareStateForArrayDestruction.");
I me
isuckatcs accepted this revision.
isuckatcs added a comment.
This revision is now accepted and ready to land.
I have nothing else to add, this patch looks good to me. If the others don't
have anything to add either, feel free to commit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST
isuckatcs updated this revision to Diff 491910.
isuckatcs marked 9 inline comments as done.
isuckatcs added a comment.
Addressed comments
Updated tests
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils
isuckatcs added a comment.
> E.g., instead of asserting true/false, checking if the assignment would
> compile.
This is actually biased. If the result of the compilation is different from the
result we get from this function, it can also mean a bug in the compiler.
Take a look at this example
isuckatcs added a comment.
This patch got a little bit out of control at the last revision, so I decided
to remove every change from clang and add everything to the `ExceptionAnalyzer`
only.
The reason for that is with exceptions we have less conversions to check than
we have inside the compile
isuckatcs updated this revision to Diff 492115.
isuckatcs added a comment.
- Reverted the changes related to clang itself
- Added more checks for exception handling
- `isQualificationConvertiblePointer` is now iterative, not recursive
- Added more test cases
CHANGES SINCE LAST ACTION
https://r
isuckatcs updated this revision to Diff 492214.
isuckatcs added a comment.
applied clang-format
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clang-tidy/ut
isuckatcs updated this revision to Diff 492215.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
clang-tools-extra/test/c
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd78a7c5012c7: [ODRHash] Handle `Integral` and `NullPtr`
template parameters in `ODRHash` (authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SIN
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:60
+// Checks if T1 is convertible to T2.
+static bool isMultiLevelConvertiblePointer(QualType P1, QualType P2,
+ unsigned CurrentLevel
isuckatcs updated this revision to Diff 489626.
isuckatcs marked 7 inline comments as done.
isuckatcs added a comment.
Addressed comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnal
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:95
+
+ if (P1->isArrayType() && P2->isArrayType()) {
+// At every level that array type is involved in, at least
xazax.hun wrote:
> isuckatcs wrote:
> > xaz
isuckatcs updated this revision to Diff 489940.
isuckatcs added a comment.
- Renamed and moved `isMultiLevelConvertiblePointer()` to `ASTContext`
- Added a unit test to test the said function
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Fil
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcaf2767c47c3: [Clang][AST] Fixed BindingDecl AST-dump for
tuple like structures (authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST A
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc6b2281bfd7: [Static Analyzer][CFG] Introducing the source
array in the CFG of… (authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Ch
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG92bf652d4074: [Static Analyzer] Small array binding policy
(authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe77ac66b8c1c: [Static Analyzer] Structured binding to data
members (authored by isuckatcs).
Herald added a subscriber: cfe-commits.
Repository:
rG
isuckatcs added a comment.
What's out desired approach for that? Create a new patch, or update this one?
Also, should I commit it as usual, or revert this commit first?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128064/new/
https://reviews.llvm
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8ef628088b54: [analyzer] Structured binding to arrays
(authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE L
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d3d5ecea5f0: [Documentation] Fixed typos in LibASTMatchers
tutorial (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:306
+
+ if (From->isMemberPointerType() || To->isMemberPointerType())
return false;
isuckatcs wrote:
> PiotrZSL wrote:
> > isuckatcs wrote:
> > > i
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp:88
+
+ for (auto [ThrowType, ThrowLoc] : AnalyzeResult.getExceptionTypes())
+diag(ThrowLoc, "may throw %0 here", DiagnosticIDs::Note)
=
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:542
+ CaughtExceptions)) {
+ CaughtExceptions.emplace(CaughtType, SourceLocation());
ExceptionInfo Rethrown = throwsExceptio
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:306
+
+ if (From->isMemberPointerType() || To->isMemberPointerType())
return false;
PiotrZSL wrote:
> isuckatcs wrote:
> > isuckatcs wrote:
> > > P
isuckatcs added a comment.
I'm not sure if I can run clang targetting AArch64 on an X86 machine but I
tried it regardless and I get compiler errors on startup, so I can't debug this
issue at the moment.
Besided that I don't know why it times out on that buildbot. The build job that
is performe
isuckatcs updated this revision to Diff 500299.
isuckatcs added a comment.
Fixed the timed out test
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clang-tid
isuckatcs added a comment.
> As long as you don't need the library (which, if you have a preprocessed file
> or a test, should absolutely not be necessary), you can just do the -cc1
> option of -triple , or the non- -cc1 option of -target .
>
> Both use basically the same options list, and that
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0303eafcb346: [clang-tidy] handle exceptions properly in
`ExceptionAnalyzer` (authored by isuckatcs).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/ne
isuckatcs added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1787
+ if (V &&
+ (!targetType->isStructureOrClassType() && !targetType->isUnionType()))
return *V;
I assume `targetType` is the type we want to interpret the re
isuckatcs added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:432
+ const LocationContext *LCtx = C.getLocationContext();
+ SVal SrcVal = State->getSVal(Buffer.Expression, LCtx);
+ QualType SourceValType = SrcVal.getType(C.getAST
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd65379c8d476: [analyzer] Remove the loop from the exploded
graph caused by missing… (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Gi
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp:84-86
+ if (AnalyzeResult.containsUnknownElements())
+diag(MatchedDecl->getLocation(), "may throw unknown exceptions",
+ DiagnosticIDs::Note);
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:390
for (const Type *T : TypesToDelete)
-ThrownExceptions.erase(T);
+ThrownExceptions.erase({T, SourceLocation()});
PiotrZSL wrote:
> isuckatcs wrot
isuckatcs added a comment.
Apart from some nits I think this patch looks good.
Comment at: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp:26
+
+AST_MATCHER(FunctionDecl, isExplicitThrow) {
+ switch (Node.getExceptionSpecType()) {
How about crea
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:319
+static bool cannotThrow(const FunctionDecl *Func) {
+ const auto *FunProto = Func->getType()->getAs();
Put this in the anonymous namespace above please t
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:160-162
+// FIXME: Two function pointers can differ in 'noexcept', but they still
+// should be considered to be same, now this triggers false-positive
because
+// T
isuckatcs added a comment.
I think the original behaviour was fine. The warning was emitted at every
occurence of the function. It might be confusing if it's only emitted for the
definition.
Also what happens in the following scenario:
int indirectly_recursive(int n) noexcept;
int recur
isuckatcs added a comment.
> "The warning was emitted at every occurence of the function. It might be
> confusing if it's only emitted for the definition."
> Why ? Issue is in definition, not declaration.
For me it would be confusing, because the forward declaration is naming the
same function
isuckatcs added inline comments.
Comment at: clang/test/Analysis/array-punned-region.c:23
+ BITFIELD_CAST *pff = &ff;
+ int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or
undefined [core.uninitialized.Assign]}}
+ return a;
@steakhal
isuckatcs added inline comments.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp,
donat.nagy, Szelethus, mikhail.ramalho.
Herald added a project: All.
Comment at: test/Analysis/casts.c:166
+ *x = 1;
+ clang_analyzer_eval(u == 1); // expected-warning{{
isuckatcs added inline comments.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411,
dkrupp, donat.nagy, baloghadamsoftware.
Herald added a project: All.
Comment at:
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:222
isuckatcs added a comment.
Please cover the changes in as much test cases as possible.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:151
+bool isFunctionTypeEqual(const FunctionType *From, const FunctionType *To) {
+ if (From == To)
Th
isuckatcs added a comment.
ping
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
isuckatcs marked an inline comment as done.
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:84
+
+QualType getPointeeOrArrayElementQualType(QualType T) {
+ if (T->isAnyPointerType())
xazax.hun wrote:
> What a
isuckatcs updated this revision to Diff 499234.
isuckatcs marked an inline comment as done.
isuckatcs added a comment.
Addressed comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnal
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:66
+ CXXBasePath &) {
+if (BS->getType()->getAsCXXRecordDecl() == BaseClass &&
+BS->getAccessSpecifier() == AS_public) {
isuckatcs updated this revision to Diff 499453.
isuckatcs added a comment.
Added tests for function pointers
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/
isuckatcs added a comment.
@xazax.hun can we merge this or should we wait for someone else's approval too?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
___
cfe-commits mailing list
cfe-commits@lis
isuckatcs updated this revision to Diff 40.
isuckatcs added a comment.
fixed failing test
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clang-tidy/util
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6b0cf1e15f8f: [clang-tidy] handle exceptions properly in
`ExceptionAnalyzer` (authored by isuckatcs).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/ne
isuckatcs added a comment.
> No because indirectly_recursive called from recursion_helper is noexcept, so
> there will be std::terminate called.
I missed that in `noexcept` functions thrown exceptions are not propagated. In
this case I agree that `recursion_helper()` shouldn't emit a warning.
isuckatcs accepted this revision.
isuckatcs added a comment.
This revision is now accepted and ready to land.
I think there's no point of holding back this patch. Even though I'm not 100%
sure we want this change, I say let's merge it and see how our users react.
It's a one line change anyway, s
isuckatcs added a comment.
Do we really want to split these two functions apart to different test files?
Is this really NFC?
The way `ExceptionEscapeCheck` works is that it creates an `ExceptionAnalyzer`
upon instantiation.
//By the way upon looking at the constructor of the check I see that
isuckatcs added a comment.
Do we really want to split these two functions apart to different test files?
Is this really NFC?
The way `ExceptionEscapeCheck` works is that it creates an `ExceptionAnalyzer`
upon instantiation.
//By the way upon looking at the constructor of the check I see that
isuckatcs added a comment.
> Yes, throw specifier is removed in C++17, split allows to support C++17 and
> above in main test file
A lot of our test files uses macros to differentiate between specific C++
standards, why not do that here too?
There are only a few occurences of functions with `th
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:306
+
+ if (From->isMemberPointerType() || To->isMemberPointerType())
return false;
isuckatcs wrote:
> Please cover this line with both positive an
isuckatcs added inline comments.
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:306
+
+ if (From->isMemberPointerType() || To->isMemberPointerType())
return false;
PiotrZSL wrote:
> isuckatcs wrote:
> > isuckatcs wrote:
> > > P
isuckatcs added a comment.
> If CFG can be updated so that the
> synthesized variables occur in the block *before* the DeclStmt containing the
> DecompositionDecl, then we can drop the workaround that is included in this
> patch.
I'm not sure that this is possible.
If you look at the AST of the
isuckatcs added a comment.
> I'm particularly interested in the case of tuple-like containers, which is a
> little different (though I think the same point holds)
`std::pair<>` is actually tuple-like if that's what you meant. Only tuple-like
types have holding vars inside a `DecompositionDecl`.
isuckatcs added a comment.
> the temporary's construction should appear before, but the binding decls,
> which use the synthetic variables, should appear after
I'm confused a bit here. Right now the CFG looks like this:
...
Based on what you say I assume you want something to happen
isuckatcs added a comment.
Those `BindingDecl`s never appear in the CFG unless one of the is used.
Take a look at the other cases.
1.) Arrays
void foo() {
int arr[2];
auto [i0, i1] = arr;
}
[B1]
1: int arr[2];
2: arr
3: [B1.2] (ImplicitCastExpr, ArrayToPoint
isuckatcs added a comment.
> Sorry, I should fix my response above: I *never* see the BindingDecls in the
> CFG, whether or not they are used.
It was my fault, I was wrong with the wording. I meant the `DeclRefExpr`s that
refer to the `BindingDecl`s. You can
access the `BindingDecl`s through t
98 matches
Mail list logo
