[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno marked an inline comment as done.
riccibruno added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2031-2033
+  "%select{|member |static member }0%2"
+  "%select{| of %3| of struct %4| of union %4| of class %4"
+  "| of structured binding %3}1">, InGroup;

rsmith wrote:
> I wonder if we could just print out a simple access path here (eg `x.a[3]`). 
> `APValue` has support for representing and pretty-printing such things 
> already, and you could try passing arbitrary lvalue expressions to the 
> constant evaluator to try to form them rather than special-casing a few 
> things in `getMemoryLocationImpl`.
Ah yes perhaps this can work. I will take a look at that. Thanks for the 
pointer!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2031-2033
+  "%select{|member |static member }0%2"
+  "%select{| of %3| of struct %4| of union %4| of class %4"
+  "| of structured binding %3}1">, InGroup;

I wonder if we could just print out a simple access path here (eg `x.a[3]`). 
`APValue` has support for representing and pretty-printing such things already, 
and you could try passing arbitrary lvalue expressions to the constant 
evaluator to try to form them rather than special-casing a few things in 
`getMemoryLocationImpl`.



Comment at: clang/lib/Sema/SemaChecking.cpp:12782-12783
+  //
+  // FIXME: Is this duplicating some code which already exists somewhere else ?
+  // If not, should this be moved somewhere where it can be reused ?
+  static MemoryLocation

Per the above, I think this can be replaced by calling `EvaluateAsLValue` on 
the expression.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 234627.
riccibruno added a comment.

Add missing patch context


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/warn-unsequenced.cpp

Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -279,17 +279,21 @@
   void member_f(S1 &s);
 };
 
+class SomeClass { public: static int x; };
+union SomeUnion { public: static int x; };
+
 void S1::member_f(S1 &s) {
-  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
-  a + ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to member 'a'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a'}}
+  a + ++a; // cxx11-warning {{unsequenced modification and access to member 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a'}}
   ++a + ++b; // no-warning
   a + ++b; // no-warning
 
-  // TODO: Warn here.
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.a + ++s.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a + ++s.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 's'}}
   ++s.a + ++s.b; // no-warning
   s.a + ++s.b; // no-warning
 
@@ -299,16 +303,18 @@
   a + ++s.b; // no-warning
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to 'bf1'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'bf1'}}
-  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to 'bf1'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'bf1'}}
+  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1'}}
+  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1'}}
   ++bf1 + ++bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   bf1 + ++bf2; // no-warning TODO {{unsequenced modification and access to}}
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++s.bf1 + ++s.bf1; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.bf1 + ++s.bf1; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.bf1 + ++s.bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1' of 's'}}
+  s.bf1 + ++s.bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1' of 's'}}
   ++s.bf1 + ++s.bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   s.bf1 + ++s.bf2; // no-warning TODO {{unsequenced modification and access to}}
 
@@ -322,19 +328,29 @@
   Der &d_ref = d;
   S1 &s1_ref = d_ref;
 
-  ++s1_ref.a + ++d_ref.a; // no-warning TODO {{multiple unsequenced modifications to member 'a' of 'd'}}
-  ++s1_ref.a + d_ref.a; // no-warning TODO {{unsequenced modification and access to member 'a' of 'd'}}
+  ++s1_ref.a + ++d_ref.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 'd'}}
+  // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 'd'}}
+  ++s1_ref.a + d_ref.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 'd'}}
+// cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 'd'}}
   ++s1_ref.a + ++d_ref.b; // no-warning
   ++s1_ref.a + d_ref.b; // no-warning
 
-  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-  ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
-  ++s.x + x; // no-warning TODO {{uns

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Note that I am not entirely sure that the implementation in 
`getMemoryLocation`/`MemoryLocation` is the right way to do this; I think that 
this patch should be considered a work-in-progress.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 234326.
riccibruno edited the summary of this revision.
riccibruno added a comment.

I have factored out various NFCs which were present in this patch. This should 
make review easier.
Also addressed some inline comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/warn-unsequenced.cpp

Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -279,17 +279,21 @@
   void member_f(S1 &s);
 };
 
+class SomeClass { public: static int x; };
+union SomeUnion { public: static int x; };
+
 void S1::member_f(S1 &s) {
-  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
-  a + ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to member 'a'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a'}}
+  a + ++a; // cxx11-warning {{unsequenced modification and access to member 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a'}}
   ++a + ++b; // no-warning
   a + ++b; // no-warning
 
-  // TODO: Warn here.
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.a + ++s.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a + ++s.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 's'}}
   ++s.a + ++s.b; // no-warning
   s.a + ++s.b; // no-warning
 
@@ -299,16 +303,18 @@
   a + ++s.b; // no-warning
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to 'bf1'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'bf1'}}
-  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to 'bf1'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'bf1'}}
+  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1'}}
+  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1'}}
   ++bf1 + ++bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   bf1 + ++bf2; // no-warning TODO {{unsequenced modification and access to}}
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++s.bf1 + ++s.bf1; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.bf1 + ++s.bf1; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.bf1 + ++s.bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1' of 's'}}
+  s.bf1 + ++s.bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1' of 's'}}
   ++s.bf1 + ++s.bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   s.bf1 + ++s.bf2; // no-warning TODO {{unsequenced modification and access to}}
 
@@ -322,19 +328,29 @@
   Der &d_ref = d;
   S1 &s1_ref = d_ref;
 
-  ++s1_ref.a + ++d_ref.a; // no-warning TODO {{multiple unsequenced modifications to member 'a' of 'd'}}
-  ++s1_ref.a + d_ref.a; // no-warning TODO {{unsequenced modification and access to member 'a' of 'd'}}
+  ++s1_ref.a + ++d_ref.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 'd'}}
+  // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 'd'}}
+  ++s1_ref.a + d_ref.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 'd'}}
+// cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 'd'}}
   ++s1_ref.a + ++d_ref.b; // no-warning
   ++s1_ref.a + d_ref.b; // no-warning
 
-  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-  ++x + x; // cxx11-warning {{unsequenced modifi

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Hello @aaron.ballman / @rsmith, can you please take a look so this patch?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D57660#1764337 , @Mordante wrote:

> I like this improvement. However I'm not a reviewer.


Thanks for looking at the patch!

> You can land some NFC changes in separate commit.

Yep, indeed. Will do when I rebase it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:12631
 
-  void VisitSequencedExpressions(Expr *SequencedBefore, Expr *SequencedAfter) {
+  void VisitSequencedExpressions(const Expr *SequencedBefore,
+ const Expr *SequencedAfter) {

You can land some NFC changes in separate commit.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I like this improvement. However I'm not a reviewer.




Comment at: clang/lib/Sema/SemaChecking.cpp:12361
+  /// \param RefsSeenPtr is used to avoid reference cycles. When such a cycle
+  /// is possible we check first if \param RefsSeenPtr is non-null. If it is
+  /// non-null we use the pointed SmallPtrSet and if null we create one on the

Please use \p here since it's not a parameter definition.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 216153.
riccibruno added a comment.

Rebased


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/warn-unsequenced.cpp

Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -160,17 +160,21 @@
   void member_f(S1 &s);
 };
 
+class SomeClass { public: static int x; };
+union SomeUnion { public: static int x; };
+
 void S1::member_f(S1 &s) {
-  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
-  a + ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to member 'a'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a'}}
+  a + ++a; // cxx11-warning {{unsequenced modification and access to member 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a'}}
   ++a + ++b; // no-warning
   a + ++b; // no-warning
 
-  // TODO: Warn here.
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.a + ++s.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a + ++s.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 's'}}
   ++s.a + ++s.b; // no-warning
   s.a + ++s.b; // no-warning
 
@@ -180,16 +184,18 @@
   a + ++s.b; // no-warning
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to 'bf1'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'bf1'}}
-  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to 'bf1'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'bf1'}}
+  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1'}}
+  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1'}}
   ++bf1 + ++bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   bf1 + ++bf2; // no-warning TODO {{unsequenced modification and access to}}
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++s.bf1 + ++s.bf1; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.bf1 + ++s.bf1; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.bf1 + ++s.bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1' of 's'}}
+  s.bf1 + ++s.bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1' of 's'}}
   ++s.bf1 + ++s.bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   s.bf1 + ++s.bf2; // no-warning TODO {{unsequenced modification and access to}}
 
@@ -203,19 +209,29 @@
   Der &d_ref = d;
   S1 &s1_ref = d_ref;
 
-  ++s1_ref.a + ++d_ref.a; // no-warning TODO {{multiple unsequenced modifications to member 'a' of 'd'}}
-  ++s1_ref.a + d_ref.a; // no-warning TODO {{unsequenced modification and access to member 'a' of 'd'}}
+  ++s1_ref.a + ++d_ref.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 'd'}}
+  // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 'd'}}
+  ++s1_ref.a + d_ref.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 'd'}}
+// cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 'd'}}
   ++s1_ref.a + ++d_ref.b; // no-warning
   ++s1_ref.a + d_ref.b; // no-warning
 
-  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-  ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
-  ++s.x + x; // no-warning TODO {{un

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D57660#1637220 , @xbolva00 wrote:

> Added some reviewers
>
> Patch improves suboptimal diagnostic, which misses bugs like:
>  https://bugs.llvm.org/show_bug.cgi?id=43052


Thanks for taking a look !


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Added some reviewers

Patch improves suboptimal diagnostic, which misses bugs like:
https://bugs.llvm.org/show_bug.cgi?id=43052


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Friendly ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.
Herald added a subscriber: dexonsmith.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Friendly ping. One thing I am wondering about is whether `MemoryLocation` and 
`getMemoryLocation` is duplicating something that is already present somewhere 
else. It feels like something similar should already exist but I can't find 
anything (but that is not saying much).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Added some context in the description of the patch. Ping !


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 188043.
riccibruno retitled this revision from "[Sema] SequenceChecker: Handle 
references and members" to "[Sema] SequenceChecker: Handle references, members 
and structured bindings.".
riccibruno edited the summary of this revision.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57660/new/

https://reviews.llvm.org/D57660

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-unsequenced.cpp

Index: test/SemaCXX/warn-unsequenced.cpp
===
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -160,17 +160,21 @@
   void member_f(S1 &s);
 };
 
+class SomeClass { public: static int x; };
+union SomeUnion { public: static int x; };
+
 void S1::member_f(S1 &s) {
-  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
-  a + ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to member 'a'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a'}}
+  a + ++a; // cxx11-warning {{unsequenced modification and access to member 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a'}}
   ++a + ++b; // no-warning
   a + ++b; // no-warning
 
-  // TODO: Warn here.
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.a + ++s.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a + ++s.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 's'}}
   ++s.a + ++s.b; // no-warning
   s.a + ++s.b; // no-warning
 
@@ -180,16 +184,18 @@
   a + ++s.b; // no-warning
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to 'bf1'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'bf1'}}
-  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to 'bf1'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'bf1'}}
+  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1'}}
+  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1'}}
   ++bf1 + ++bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   bf1 + ++bf2; // no-warning TODO {{unsequenced modification and access to}}
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++s.bf1 + ++s.bf1; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.bf1 + ++s.bf1; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.bf1 + ++s.bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1' of 's'}}
+  s.bf1 + ++s.bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1' of 's'}}
   ++s.bf1 + ++s.bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   s.bf1 + ++s.bf2; // no-warning TODO {{unsequenced modification and access to}}
 
@@ -203,19 +209,29 @@
   Der &d_ref = d;
   S1 &s1_ref = d_ref;
 
-  ++s1_ref.a + ++d_ref.a; // no-warning TODO {{multiple unsequenced modifications to member 'a' of 'd'}}
-  ++s1_ref.a + d_ref.a; // no-warning TODO {{unsequenced modification and access to member 'a' of 'd'}}
+  ++s1_ref.a + ++d_ref.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 'd'}}
+  // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 'd'}}
+  ++s1_ref.a + d_ref.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 'd'}}
+// cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 'd'}}
   ++s1_ref.a + ++d_ref.b; // no-warning
   ++s1_ref.a + d_ref.b; // no-warning
 
-  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-  ++x + x; // cxx11-warning {{unsequenced mo