[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2018-03-17 Thread Zhihao Yuan via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC327782: [C++17] Allow an empty expression in an if init 
statement (authored by lichray, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp

Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -Wc++14-compat -verify %s -DCPP17
 
 int f();
 
@@ -10,10 +11,67 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+void ifInitStatement() {
+  int Var = 0;
+
+  if (int I = 0; true) {}
+  if (Var + Var; true) {}
+  if (; true) {}
+#ifdef CPP17
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+#endif
+}
+
+void switchInitStatement() {
+  int Var = 0;
+
+  switch (int I = 0; Var) {}
+  switch (Var + Var; Var) {}
+  switch (; Var) {}
+#ifdef CPP17
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+#endif
+}
+
+// TODO: Better diagnostics for while init statements.
+void whileInitStatement() {
+  while (int I = 10; I--); // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{use of undeclared identifier 'I'}}
+
+  int Var = 10;
+  while (Var + Var; Var--) {} // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+  // expected-warning@-4 {{while loop has empty body}}
+  // expected-note@-5 {{put the semicolon on a separate line to silence this warning}}
+}
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {
+  while (; false) {} // expected-error {{expected expression}}
+  // expected-warning@-1 {{expression result unused}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+}
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1744,17 +1744,34 @@
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  const auto WarnOnInit = [this, ] {
+Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
+? diag::warn_cxx14_compat_init_statement
+: diag::ext_init_statement)
+<< (CK == Sema::ConditionKind::Switch);
+  };
+
   // Determine what kind of thing we have.
   switch (isCXXConditionDeclarationOrInitStatement(InitStmt)) {
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
+  SourceLocation SemiLoc = ConsumeToken();
+  *InitStmt = Actions.ActOnNullStmt(SemiLoc);
+  return ParseCXXCondition(nullptr, Loc, CK);
+}
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
   return Sema::ConditionError();
 
 if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
   *InitStmt = Actions.ActOnExprStmt(Expr.get());
   ConsumeToken();
   return ParseCXXCondition(nullptr, Loc, CK);
@@ -1764,10 +1781,7 @@
   }
 
   case ConditionOrInitStatement::InitStmtDecl: {
-Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
-?

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2018-03-14 Thread Nicolas Lesser via Phabricator via cfe-commits 
Rakete added a comment.

@rsmith Can you commit please?


https://reviews.llvm.org/D40445



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

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2018-03-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM


https://reviews.llvm.org/D40445



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

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2018-03-14 Thread Nicolas Lesser via Phabricator via cfe-commits 
Rakete updated this revision to Diff 138339.
Rakete added a comment.

Rebase and ping :)


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp

Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -Wc++14-compat -verify %s -DCPP17
 
 int f();
 
@@ -10,10 +11,67 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+void ifInitStatement() {
+  int Var = 0;
+
+  if (int I = 0; true) {}
+  if (Var + Var; true) {}
+  if (; true) {}
+#ifdef CPP17
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+#endif
+}
+
+void switchInitStatement() {
+  int Var = 0;
+
+  switch (int I = 0; Var) {}
+  switch (Var + Var; Var) {}
+  switch (; Var) {}
+#ifdef CPP17
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+#endif
+}
+
+// TODO: Better diagnostics for while init statements.
+void whileInitStatement() {
+  while (int I = 10; I--); // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{use of undeclared identifier 'I'}}
+
+  int Var = 10;
+  while (Var + Var; Var--) {} // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+  // expected-warning@-4 {{while loop has empty body}}
+  // expected-note@-5 {{put the semicolon on a separate line to silence this warning}}
+}
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {
+  while (; false) {} // expected-error {{expected expression}}
+  // expected-warning@-1 {{expression result unused}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+}
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1744,17 +1744,34 @@
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  const auto WarnOnInit = [this, ] {
+Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
+? diag::warn_cxx14_compat_init_statement
+: diag::ext_init_statement)
+<< (CK == Sema::ConditionKind::Switch);
+  };
+
   // Determine what kind of thing we have.
   switch (isCXXConditionDeclarationOrInitStatement(InitStmt)) {
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
+  SourceLocation SemiLoc = ConsumeToken();
+  *InitStmt = Actions.ActOnNullStmt(SemiLoc);
+  return ParseCXXCondition(nullptr, Loc, CK);
+}
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
   return Sema::ConditionError();
 
 if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
   *InitStmt = Actions.ActOnExprStmt(Expr.get());
   ConsumeToken();
   return ParseCXXCondition(nullptr, Loc, CK);
@@ -1764,10 +1781,7 @@
   }
 
   case ConditionOrInitStatement::InitStmtDecl: {
-Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
-? diag::warn_cxx14_compat_init_statement
-: diag::ext_init_statement)
-<< (CK ==

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2018-01-05 Thread Nicolas Lesser via Phabricator via cfe-commits 
Rakete updated this revision to Diff 128729.
Rakete added a comment.

Addressed review comments. I'll write up a patch in the next coming days then :)


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp

Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -Wc++14-compat -verify %s -DCPP17
 
 int f();
 
@@ -10,10 +11,68 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+void ifInitStatement() {
+  int Var = 0;
+
+  if (int I = 0; true) {}
+  if (Var + Var; true) {}
+  if (; true) {}
+#ifdef CPP17
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+#endif
+}
+
+void switchInitStatement() {
+  int Var = 0;
+
+  switch (int I = 0; Var) {}
+  switch (Var + Var; Var) {}
+  switch (; Var) {}
+#ifdef CPP17
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+#endif
+}
+
+// TODO: Better diagnostics for while init statements.
+void whileInitStatement() {
+  while (int I = 10; I--); // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{use of undeclared identifier 'I'}}
+
+  int Var = 10;
+  while (Var + Var; Var--) {} // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+  // expected-warning@-4 {{while loop has empty body}}
+  // expected-note@-5 {{put the semicolon on a separate line to silence this warning}}
+}
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {
+  while (; false) {} // expected-error {{expected expression}}
+  // expected-warning@-1 {{expression result unused}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+}
+
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1742,17 +1742,34 @@
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  const auto WarnOnInit = [this, ] {
+Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
+? diag::warn_cxx14_compat_init_statement
+: diag::ext_init_statement)
+<< (CK == Sema::ConditionKind::Switch);
+  };
+
   // Determine what kind of thing we have.
   switch (isCXXConditionDeclarationOrInitStatement(InitStmt)) {
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
+  SourceLocation SemiLoc = ConsumeToken();
+  *InitStmt = Actions.ActOnNullStmt(SemiLoc);
+  return ParseCXXCondition(nullptr, Loc, CK);
+}
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
   return Sema::ConditionError();
 
 if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
   *InitStmt = Actions.ActOnExprStmt(Expr.get());
   ConsumeToken();
   return ParseCXXCondition(nullptr, Loc, CK);
@@ -1762,10 +1779,7 @@
   }
 
   case ConditionOrInitStatement::InitStmtDecl: {
-Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
-? diag::warn_cxx14_compat_init_statement
-

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2018-01-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added inline comments.



Comment at: lib/Parse/ParseExprCXX.cpp:1760
+if (InitStmt && TryConsumeToken(tok::semi)) {
+  WarnOnInit();
+  return ParseCXXCondition(nullptr, Loc, CK);

Please create a NullStmt here (`Actions.ActOnNullStmt`), rather than producing 
an AST that's indistinguishable from one where the init statement was omitted.



Comment at: lib/Parse/ParseExprCXX.cpp:1780-1783
 Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
 ? diag::warn_cxx14_compat_init_statement
 : diag::ext_init_statement)
 << (CK == Sema::ConditionKind::Switch);

Use `WarnOnInit()` here.



Comment at: test/CXX/stmt.stmt/stmt.select/p3.cpp:70-71
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax 
after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {

I think error recovery thought that a `)` was omitted before the `;`, so it 
imagined we had:

 ```
  while (Var + Var); Var--) {}
```

Then it thought that a `;` was omitted before the `)`, so it imagined we had:

 ```
  while (Var + Var); Var--; ) {}
```

Then it saw a statement beginning with a `)`, and decided to skip to the next 
statement, which would skip past the `)`, the `{`, the `}`, and whatever 
statement you put next in the function.

Maybe we should parse as if an //init-statement// were permitted on a `while` 
loop (and produce an error after the fact if there's one present). But that 
should be done in a separate patch if you're interested in working on it.


https://reviews.llvm.org/D40445



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

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2018-01-04 Thread Nicolas Lesser via Phabricator via cfe-commits 
Rakete updated this revision to Diff 128659.
Rakete added a comment.

Rebased + friendly 2018 ping :)


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp

Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -Wc++14-compat -verify %s -DCPP17
 
 int f();
 
@@ -10,10 +11,67 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+void ifInitStatement() {
+  int Var = 0;
+
+  if (int I = 0; true) {}
+  if (Var + Var; true) {}
+  if (; true) {}
+#ifdef CPP17
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+#endif
+}
+
+void switchInitStatement() {
+  int Var = 0;
+
+  switch (int I = 0; Var) {}
+  switch (Var + Var; Var) {}
+  switch (; Var) {}
+#ifdef CPP17
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+#endif
+}
+
+// TODO: Better diagnostics for while init statements.
+void whileInitStatement() {
+  while (int I = 10; I--); // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{use of undeclared identifier 'I'}}
+
+  int Var = 10;
+  while (Var + Var; Var--) {} // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+  // expected-warning@-4 {{while loop has empty body}}
+  // expected-note@-5 {{put the semicolon on a separate line to silence this warning}}
+}
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {
+  while (; false) {} // expected-error {{expected expression}}
+  // expected-warning@-1 {{expression result unused}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+}
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1742,17 +1742,32 @@
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  const auto WarnOnInit = [this, ] {
+Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
+? diag::warn_cxx14_compat_init_statement
+: diag::ext_init_statement)
+<< (CK == Sema::ConditionKind::Switch);
+  };
+
   // Determine what kind of thing we have.
   switch (isCXXConditionDeclarationOrInitStatement(InitStmt)) {
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (InitStmt && TryConsumeToken(tok::semi)) {
+  WarnOnInit();
+  return ParseCXXCondition(nullptr, Loc, CK);
+}
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
   return Sema::ConditionError();
 
 if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
   *InitStmt = Actions.ActOnExprStmt(Expr.get());
   ConsumeToken();
   return ParseCXXCondition(nullptr, Loc, CK);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2017-11-24 Thread Nicolas Lesser via Phabricator via cfe-commits 
Rakete updated this revision to Diff 124239.
Rakete edited the summary of this revision.
Rakete added a comment.

Added more test coverage for compatibility warnings, and fixed a bug at the 
same time :).


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp

Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -Wc++14-compat -verify %s -DCPP17
 
 int f();
 
@@ -10,10 +11,68 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+void ifInitStatement() {
+  int Var = 0;
+
+  if (int I = 0; true) {}
+  if (Var + Var; true) {}
+  if (; true) {}
+#ifdef CPP17
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+#endif
+}
+
+void switchInitStatement() {
+  int Var = 0;
+
+  switch (int I = 0; Var) {}
+  switch (Var + Var; Var) {}
+  switch (; Var) {}
+#ifdef CPP17
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+#endif
+}
+
+// TODO: Better diagnostics for while init statements.
+void whileInitStatement() {
+  while (int I = 10; I--); // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{use of undeclared identifier 'I'}}
+
+  int Var = 10;
+  while (Var + Var; Var--) {} // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+  // expected-warning@-4 {{while loop has empty body}}
+  // expected-note@-5 {{put the semicolon on a separate line to silence this warning}}
+}
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {
+  while (; false) {} // expected-error {{expected expression}}
+  // expected-warning@-1 {{expression result unused}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+}
+
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1740,17 +1740,32 @@
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  const auto WarnOnInit = [this, ] {
+Diag(Tok.getLocation(), getLangOpts().CPlusPlus1z
+? diag::warn_cxx14_compat_init_statement
+: diag::ext_init_statement)
+<< (CK == Sema::ConditionKind::Switch);
+  };
+
   // Determine what kind of thing we have.
   switch (isCXXConditionDeclarationOrInitStatement(InitStmt)) {
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (InitStmt && TryConsumeToken(tok::semi)) {
+  WarnOnInit();
+  return ParseCXXCondition(nullptr, Loc, CK);
+}
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
   return Sema::ConditionError();
 
 if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
   *InitStmt = Actions.ActOnExprStmt(Expr.get());
   ConsumeToken();
   return ParseCXXCondition(nullptr, Loc, CK);
@@ -1760,10 +1775,7 @@
   }
 
   case ConditionOrInitStatement::InitStmtDecl: {
-Diag(Tok.getLocation(), getLangOpts().CPlusPlus1z
-? diag::warn_cxx14_compat_init_statement
-: diag::ext_init_statement)
-

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2017-11-24 Thread Nicolas Lesser via Phabricator via cfe-commits 
Rakete updated this revision to Diff 124235.
Rakete added a comment.

Added a test for the switch statement and added full context to the diff.


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -10,10 +11,41 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int ifInitStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+
+int switchInitStatement() {
+  switch (int I = 1; I) {
+  case 1:
+return I;
+  }
+
+  int Var = 0;
+  switch (Var + Var; Var) {
+  case 0:
+return Var;
+  }
+
+  switch (; 0) {
+  case 0:
+return 0;
+  }
+}
+#endif
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -10,10 +11,41 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int ifInitStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+
+int switchInitStatement() {
+  switch (int I = 1; I) {
+  case 1:
+return I;
+  }
+
+  int Var = 0;
+  switch (Var + Var; Var) {
+  case 0:
+return Var;
+  }
+
+  switch (; 0) {
+  case 0:
+return 0;
+  }
+}
+#endif
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2017-11-24 Thread Erik Pilkington via Phabricator via cfe-commits 
erik.pilkington added a comment.

Hi, thanks for working on this!
Can you add tests to make sure that this also works with switch statements 
(which also have this bug), and not with while?  Also, it makes it a lot easier 
to review these patches if you add context lines to the diff.
Thanks,
Erik




Comment at: lib/Parse/ParseExprCXX.cpp:1750
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);

We should only be doing this if InitStmt != nullptr, right? I think this would 
lead us to be too permissive with while statements, which don't have this 
feature. Also, it would be nice to emit a c++14-compat warning here, like below.


https://reviews.llvm.org/D40445



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

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

2017-11-24 Thread Nicolas Lesser via Phabricator via cfe-commits 
Rakete created this revision.
Rakete added a project: clang.

This fixes PR35381 .


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -17,3 +18,18 @@
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int initStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+#endif
+
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -17,3 +18,18 @@
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int initStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+#endif
+
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits