https://github.com/dtarditi commented:
@ziqingluo-90 @malavikasamak thanks for adding me to this review. I left one
informational comment and one piece of feedback.
https://github.com/llvm/llvm-project/pull/114894
___
cfe-commits mailing list
cfe-co
@@ -353,23 +355,90 @@ isInUnspecifiedUntypedContext(internal::Matcher
InnerMatcher) {
return stmt(anyOf(CompStmt, IfStmtThen, IfStmtElse));
}
+// Returns true iff integer E1 is equivalent to integer E2.
+//
+// For now we only support such expressions:
+//expr := DRE |
https://github.com/dtarditi edited
https://github.com/llvm/llvm-project/pull/114894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dtarditi wrote:
I agree that it is fine to not handle this for now.
If we do decide to handle this, we need to be careful because this equivalence
does not hold for signed integer arithmetic expressions in C. The equivalence
relies on commutativity and as
https://github.com/dtarditi approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/129169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dtarditi wrote:
Thanks - I updated the PR title and description so that it can be used for the
commit.
It could be a little confusing to read the PR thread with the updated
description. Here is the original description, in case anyone reading the
thread needs the context.
> A clang user poi
https://github.com/dtarditi edited
https://github.com/llvm/llvm-project/pull/126596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dtarditi wrote:
Thanks everyone for the reviews and feedback.
I don't have write access for LLVM. @NagyDonat or @haoNoQ, could you merge it
on my behalf? Do you want me to condense the change to a single commit as
recommended [here](https://llvm.org/docs/GitHub.html#landing-your-change)?
A
https://github.com/dtarditi approved this pull request.
Looks good - thank you for adding the tests.
https://github.com/llvm/llvm-project/pull/125671
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listin
https://github.com/dtarditi updated
https://github.com/llvm/llvm-project/pull/126596
>From 06eb6682196249f4cae9801963380d0881d27296 Mon Sep 17 00:00:00 2001
From: David Tarditi
Date: Mon, 10 Feb 2025 11:35:45 -0800
Subject: [PATCH 1/3] [analyzer] Update undefined assignment diagnostics to not
dtarditi wrote:
@haoNoQ I think sticking with uninitialized is good. I've updated the patch
with new error messages using that. Please take a look at it and let me know
what you think. I think the right path is to issue a more precise error
message for out-of-bounds reads. As you point, w
https://github.com/dtarditi updated
https://github.com/llvm/llvm-project/pull/126596
>From 06eb6682196249f4cae9801963380d0881d27296 Mon Sep 17 00:00:00 2001
From: David Tarditi
Date: Mon, 10 Feb 2025 11:35:45 -0800
Subject: [PATCH 1/3] [analyzer] Update undefined assignment diagnostics to not
dtarditi wrote:
Also, please avoid force pushes if you can. It confuses the PR history.
https://github.com/llvm/llvm-project/pull/125671
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
dtarditi wrote:
@haoNoQ thanks for the explanation! Yes, this makes sense. I agree that the
right long-term approach is that the ArrayBoundChecker issues warnings about
out-of-bounds accesses. In general, I think it is desirable to report a
problem about undefined behavior as close to its ca
dtarditi wrote:
@haoNoQ I believe the static analyzer can produce undefined values from
out-of-bounds memory accesses also. There are some test cases in this change
that show that.
Some possible error message that I think cover both cases are:
- `use of uninitialized memory or out-of-bounds
dtarditi wrote:
@haoNoQ Thank you for the welcome and the helpful information!
Your suggestion sounds good to me. I had actually started down this route
initially of pointing out the logical error of using a variable before it is
initialized. I changed direction because I was not sure where
dtarditi wrote:
@steakhal Yes, the term garbage has a negative connotation when used as an
adjective in English. By a little snarky, the user meant that the message
could be seen as a little overly critical. The user suggested using more
neutral terminology.
https://github.com/llvm/llvm-p
dtarditi wrote:
@Xazax-hun and @haoNoQ. Could one of you review this or assign it to someone
to review? Thanks.
https://github.com/llvm/llvm-project/pull/126596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin
https://github.com/dtarditi created
https://github.com/llvm/llvm-project/pull/126596
A clang user pointed out that messages for the static analyzer undefined
assignment checker use the term 'garbage'. This is kind of snarky and also
imprecise. This change replaces the term 'garbage' in those
dtarditi updated this revision to Diff 78375.
dtarditi added a comment.
The parameter array needed to be initialized so that assignments involving
unique pointers work properly. The memory could be uninitialized according to
C++ semantics.. Visual C++ was zeroing the memory and GCC was not. T
dtarditi added a comment.
I sync'ed to the head of the tree. I tried to reproduce the failures that you
saw and couldn't.I'm building on Windows 10 x64 using Visual Studio. What
platform/OS did you build on? I built and test both Debug and RelWithDebugInfo.
Here's what I saw for RelWithD
dtarditi updated this revision to Diff 77373.
dtarditi added a comment.
Thanks for the code review feedback - I've addressed it. Yes, we should use
reset() instead of release(). I also deleted the unnecessary brackets. I
don't have commit access, so if this looks good, could someone commit t
dtarditi created this revision.
dtarditi added a subscriber: cfe-commits.
This changes pointers to cached tokens for default arguments in C++ from raw
pointers to unique_ptrs. There was a fixme in the code where the cached tokens
are created about using a smart pointer.
The change is straight
dtarditi created this revision.
dtarditi added a reviewer: erik.pilkington.
dtarditi added a subscriber: cfe-commits.
r272587 (http://reviews.llvm.org/D20490) fixed an issue where typo correction
could cause a crash when compiling C programs.The problem was that a typo
expression could be in
r272587 (http://reviews.llvm.org/D20490) fixed an issue where typo correction
could cause a crash when compiling C programs.The problem was that a typo
expression could be inadvertently processed twice.r272587 fixed this for
BinOp expressions. Conditional expressions can hit the same p
25 matches
Mail list logo
