This revision was automatically updated to reflect the committed changes.
Closed by commit rL347258: [clang][CodeGen] Implicit Conversion Sanitizer:
discover the world of… (authored by lebedevri, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1303154, @rjmccall wrote:
> Heh, okay.
Okay, great, thank you for the review!
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Heh, okay.
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1296232, @rjmccall wrote:
> Seems reasonable to me.
I'm very happy that it seems reasonable.
I'm waiting for either review comments, or formal approval :)
Ping.
Repository:
rC Clang
https://reviews.llvm.org/D53949
rjmccall added a comment.
Seems reasonable to me.
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added a comment.
Ping.
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri updated this revision to Diff 172335.
lebedev.ri added a comment.
And finish the test coverage.
(sidenote: will these follow-ups again be stuck for two months? let's see!)
Repository:
rC Clang
https://reviews.llvm.org/D53949
Files:
lib/CodeGen/CGExprScalar.cpp
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1283872, @regehr wrote:
> Regarding ++ and --, I think for now it's fine to just document that these
> aren't instrumented.
Indeed, that is a different issue, i don't want to solve it in this
differential.
Though, there currently
regehr added a comment.
Regarding ++ and --, I think for now it's fine to just document that these
aren't instrumented.
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
lebedev.ri updated this revision to Diff 172115.
lebedev.ri edited the summary of this revision.
lebedev.ri added a subscriber: mclow.lists.
lebedev.ri added a comment.
Added test coverage.
Please review :)
Repository:
rC Clang
https://reviews.llvm.org/D53949
Files:
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1282884, @regehr wrote:
> I do not agree that ++ is performed on the original type. The C99 standard
> (6.5.3.1.2) appears to be very clear on this point: "The expression ++E is
> equivalent to (E+=1)."
In
rsmith added a comment.
> In https://reviews.llvm.org/D53949#1282884, @regehr wrote:
>
>> The C99 standard (6.5.3.1.2) appears to be very clear on this point: "The
>> expression ++E is equivalent to (E+=1)."
>
>
> That is clearly not what clang is doing here.
How so? E += 1 is equivalent to
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1282884, @regehr wrote:
> I do not agree that ++ is performed on the original type.
I was only talking about the IR.
In https://reviews.llvm.org/D53949#1282884, @regehr wrote:
> The C99 standard (6.5.3.1.2) appears to be very
regehr added a comment.
I do not agree that ++ is performed on the original type. The C99 standard
(6.5.3.1.2) appears to be very clear on this point: "The expression ++E is
equivalent to (E+=1)."
Repository:
rC Clang
https://reviews.llvm.org/D53949
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1282870, @lebedev.ri wrote:
> In https://reviews.llvm.org/D53949#1282866, @regehr wrote:
>
> > This patch doesn't appear to yet fix the "x++" or "x--" cases.
>
>
> It won't, the increment/decrement happens on the original type, there
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1282866, @regehr wrote:
> This patch doesn't appear to yet fix the "x++" or "x--" cases.
It won't, the increment/decrement happens on the original type, there is no
cast there. https://godbolt.org/z/WuWA62
Those cases are for
regehr added a comment.
This patch doesn't appear to yet fix the "x++" or "x--" cases.
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
lebedev.ri added a comment.
In https://reviews.llvm.org/D53949#1282861, @regehr wrote:
> I can test this and write a few test cases.
I'll write the tests tomorrow, i just wanted to post the initial code diff.
(it is a shame that there isn't any working analog of
rsmith added a comment.
Looks good once some tests are added.
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
regehr added a comment.
I can test this and write a few test cases.
Repository:
rC Clang
https://reviews.llvm.org/D53949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, regehr, vsk, rjmccall, Sanitizers.
lebedev.ri added projects: clang, Sanitizers.
Not yet for an //actual// review, needs tests.
As reported by @regehr (thanks!) on twitter
21 matches
Mail list logo
