[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: lib/Sema/SemaLambda.cpp:1491
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();

dim wrote:
> Just for sanity's sake, I would still put an assert here, that 
> `getVariable()` does not return `nullptr`.  That might catch similar issues 
> in the future.  (And it is better than a segfault... :) )
> 
I'm adding an assert to `getVariable()` instead.


Repository:
  rC Clang

https://reviews.llvm.org/D41016



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

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Malcolm Parsons via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320396: [Sema] Fix crash in unused-lambda-capture warning 
for VLAs (authored by malcolm.parsons).

Repository:
  rL LLVM

https://reviews.llvm.org/D41016

Files:
  cfe/trunk/include/clang/Sema/ScopeInfo.h
  cfe/trunk/lib/Sema/SemaLambda.cpp
  cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp


Index: cfe/trunk/include/clang/Sema/ScopeInfo.h
===
--- cfe/trunk/include/clang/Sema/ScopeInfo.h
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h
@@ -560,6 +560,7 @@
 void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; }
 
 VarDecl *getVariable() const {
+  assert(isVariableCapture());
   return VarAndNestedAndThis.getPointer();
 }
 
Index: cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp
+++ cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function 
template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not 
used}}
+}
+} // namespace pr3
Index: cfe/trunk/lib/Sema/SemaLambda.cpp
===
--- cfe/trunk/lib/Sema/SemaLambda.cpp
+++ cfe/trunk/lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";


Index: cfe/trunk/include/clang/Sema/ScopeInfo.h
===
--- cfe/trunk/include/clang/Sema/ScopeInfo.h
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h
@@ -560,6 +560,7 @@
 void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; }
 
 VarDecl *getVariable() const {
+  assert(isVariableCapture());
   return VarAndNestedAndThis.getPointer();
 }
 
Index: cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp
+++ cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}}
+}
+} // namespace pr3
Index: cfe/trunk/lib/Sema/SemaLambda.cpp
===
--- cfe/trunk/lib/Sema/SemaLambda.cpp
+++ cfe/trunk/lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons updated this revision to Diff 126394.
malcolm.parsons marked an inline comment as done.
malcolm.parsons added a comment.

Add assert.


Repository:
  rC Clang

https://reviews.llvm.org/D41016

Files:
  include/clang/Sema/ScopeInfo.h
  lib/Sema/SemaLambda.cpp
  test/SemaCXX/warn-unused-lambda-capture.cpp


Index: test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- test/SemaCXX/warn-unused-lambda-capture.cpp
+++ test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function 
template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not 
used}}
+}
+} // namespace pr3
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";
Index: include/clang/Sema/ScopeInfo.h
===
--- include/clang/Sema/ScopeInfo.h
+++ include/clang/Sema/ScopeInfo.h
@@ -560,6 +560,7 @@
 void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; }
 
 VarDecl *getVariable() const {
+  assert(isVariableCapture());
   return VarAndNestedAndThis.getPointer();
 }
 


Index: test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- test/SemaCXX/warn-unused-lambda-capture.cpp
+++ test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}}
+}
+} // namespace pr3
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";
Index: include/clang/Sema/ScopeInfo.h
===
--- include/clang/Sema/ScopeInfo.h
+++ include/clang/Sema/ScopeInfo.h
@@ -560,6 +560,7 @@
 void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; }
 
 VarDecl *getVariable() const {
+  assert(isVariableCapture());
   return VarAndNestedAndThis.getPointer();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D41016



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

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Dimitry Andric via Phabricator via cfe-commits]

dim accepted this revision.
dim added a comment.
This revision is now accepted and ready to land.

For the rest, LGTM.  It fixes the segfault, for both the full original test 
case, and my reduced version.




Comment at: lib/Sema/SemaLambda.cpp:1491
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();

Just for sanity's sake, I would still put an assert here, that `getVariable()` 
does not return `nullptr`.  That might catch similar issues in the future.  
(And it is better than a segfault... :) )



Repository:
  rC Clang

https://reviews.llvm.org/D41016



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

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons created this revision.
Herald added a subscriber: cfe-commits.

Clang was crashing when diagnosing an unused-lambda-capture for a VLA because
From.getVariable() is null for the capture of a VLA bound.
Warning about the VLA bound capture is not helpful, so only warn for the VLA
itself.

Fixes: PR3


Repository:
  rC Clang

https://reviews.llvm.org/D41016

Files:
  lib/Sema/SemaLambda.cpp
  test/SemaCXX/warn-unused-lambda-capture.cpp


Index: test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- test/SemaCXX/warn-unused-lambda-capture.cpp
+++ test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function 
template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not 
used}}
+}
+} // namespace pr3
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";


Index: test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- test/SemaCXX/warn-unused-lambda-capture.cpp
+++ test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}}
+}
+} // namespace pr3
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits