stryku added a comment.
Hi again, since I don't have commit rights, could you please merge this patch
into the project sources? Thanks in advance.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D50766/new/
https://reviews.llvm.org/D50766
___
stryku added a comment.
@rsmith Of course I would like. Not sure what's better: to submit one patch per
case or submit all of them in one patch (didn't deal much with them, not sure
how big will the change be)?
https://reviews.llvm.org/D50766
___
stryku marked an inline comment as done.
stryku added inline comments.
Comment at: test/SemaCXX/warn-unsequenced-cxx17.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wno-unused %s
+
lebedev.ri wrote:
> Rakete wrote:
> > lebedev.ri wrote:
> > > On
stryku added a comment.
@Rakete Any more comments?
https://reviews.llvm.org/D50766
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
stryku added a comment.
Friendly ping (:
https://reviews.llvm.org/D50766
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
stryku marked an inline comment as done.
stryku added a comment.
Not sure if you guys are still reviewing this. Do you have any more thoughts?
If no, what should I do to move further with the patch?
https://reviews.llvm.org/D50766
___
cfe-commits m
stryku marked 2 inline comments as done.
stryku added a comment.
@Rakete or anyone else, any more comments? (:
Comment at: test/SemaCXX/warn-unsequenced-cxx17.cpp:7
+ // expected-no-diagnostics
+ p[(long long unsigned)(p = 0)] // ok
+}
Rakete wrote:
>
stryku updated this revision to Diff 162630.
stryku added a comment.
Added missing semicolon.
https://reviews.llvm.org/D50766
Files:
lib/Sema/SemaChecking.cpp
test/SemaCXX/warn-unsequenced-cxx17.cpp
test/SemaCXX/warn-unsequenced.cpp
Index: test/SemaCXX/warn-unsequenced.cpp
=
stryku updated this revision to Diff 162585.
stryku added a comment.
Added test cases for the code.
Here I think I need your assistance. AFAIK in C++ 17 a couple more sequence
related rules appeared. If we would like to cover them in current
//warn-unsequenced.cpp// file, I think we would end u
stryku added a comment.
Thanks for the advice. Should I update a new diff with `-U9`?
About tests. I've probably missed something but I looked through the project
and didn't find any tests for unsequenced operations. If you know a place where
such tests are placed, please point it and I'll
stryku updated this revision to Diff 161943.
stryku added a comment.
@Rakete Thank you for the comments. You're perfectly right, there was an
issue in my code. I've fixed it and also E1 and E2 will be sequenced only in
C++ >= 17.
https://reviews.llvm.org/D50766
Files:
lib/Sema/SemaCheck
stryku updated this revision to Diff 160885.
stryku added a comment.
Thanks for pointing that out. You're probably right, these two calls are
self-explanatory.
https://reviews.llvm.org/D50766
Files:
lib/Sema/SemaChecking.cpp
Index: lib/Sema/SemaChecking.cpp
stryku created this revision.
stryku added a reviewer: rsmith.
stryku added a project: clang.
In the [expr.sub] p1, we can read that for a given E1[E2], E1 is sequenced
before E2.
Repository:
rC Clang
https://reviews.llvm.org/D50766
Files:
lib/Sema/SemaChecking.cpp
Index: lib/Sema/SemaC
stryku added a comment.
@djasper You're probably right. Thanks anyway for your time (:
Repository:
rL LLVM
https://reviews.llvm.org/D29039
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-
14 matches
Mail list logo
