[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

2018-04-16 Thread easyaspi314 (Devin) via Phabricator via cfe-commits 
easyaspi314 added inline comments.



Comment at: bindings/python/tests/cindex/test_diagnostics.py:18
 self.assertEqual(tu.diagnostics[0].spelling,
-'control reaches end of non-void function')
+'non-void function does not return a value')
 

Quuxplusone wrote:
> rsmith wrote:
> > It seems like part of the problem here is that "non-void function" is 
> > sort-of nonsense due to a few missing words. How about:
> > 
> > > "control can reach end of function with non-void return type"
> > 
> > or similar?
> > 
> > I think we still need the mention of control flow, because we're *not* 
> > saying the function contains no return statements, we're saying there's a 
> > control flow path that reaches the end of the function.
> I think OP's issue here is that the current message about "control" is 
> intrinsically confusing. //Of course// "control can reach end of function"; 
> if control never reaches the end of the function, you must have an infinite 
> loop somewhere! The important missing piece of the current message is that 
> control reaches the end of the function //without encountering any return 
> statement//.
> 
> > we're *not* saying the function contains no return statements
> 
> Not sure, but I think in this case we //are// saying that. There's a 
> different message, "non-void function //might// not return a value," for 
> those other cases.
> 
> However, if I'm wrong about that, then another wording option close to OP's 
> suggestion would be "non-void function does not //always// return a value" / 
> "non-void function might not //always// return a value."
> I think we still need the mention of control flow.

I see where you are going.

However, I have another idea, but I can't implement it myself.

What if we give the user a note like this:

```
int someCondition;

int a() {
if (someCondition) {
return 3;
}
}
```
```
foo.c:3:1: warning: non-void function might not return a value [-Wreturn-type]
int a() {
^
foo.c:4:5: note: assuming 'someCondition' is false
if (someCondition) {
^
foo.c:7:1: note: control flow reaches end of function without a return value
}
^
1 warning generated.
```



Repository:
  rC Clang

https://reviews.llvm.org/D45643



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

[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

2018-04-16 Thread easyaspi314 (Devin) via Phabricator via cfe-commits 
easyaspi314 added inline comments.



Comment at: docs/DiagnosticsReference.rst:9097
 |   |++|   
  |
 
+---+--+-+

rsmith wrote:
> This is a generated file; please don't manually update it. (Though we should 
> regenerate it, it's probably quite stale...)
Serious question though, why in the world do we have generated files in an open 
source repo? 

Isn't is as stupid as putting the precompiled binaries in the source tree? I'd 
rather wait a few minutes to generate some files than spend 4 hours trying to 
figure out how to deal with serialized-diags-stable.dia.


Repository:
  rC Clang

https://reviews.llvm.org/D45643



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

[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

2018-04-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits 
Quuxplusone added inline comments.



Comment at: bindings/python/tests/cindex/test_diagnostics.py:18
 self.assertEqual(tu.diagnostics[0].spelling,
-'control reaches end of non-void function')
+'non-void function does not return a value')
 

rsmith wrote:
> It seems like part of the problem here is that "non-void function" is sort-of 
> nonsense due to a few missing words. How about:
> 
> > "control can reach end of function with non-void return type"
> 
> or similar?
> 
> I think we still need the mention of control flow, because we're *not* saying 
> the function contains no return statements, we're saying there's a control 
> flow path that reaches the end of the function.
I think OP's issue here is that the current message about "control" is 
intrinsically confusing. //Of course// "control can reach end of function"; if 
control never reaches the end of the function, you must have an infinite loop 
somewhere! The important missing piece of the current message is that control 
reaches the end of the function //without encountering any return statement//.

> we're *not* saying the function contains no return statements

Not sure, but I think in this case we //are// saying that. There's a different 
message, "non-void function //might// not return a value," for those other 
cases.

However, if I'm wrong about that, then another wording option close to OP's 
suggestion would be "non-void function does not //always// return a value" / 
"non-void function might not //always// return a value."



Comment at: include/clang/Basic/DiagnosticASTKinds.td:33
 def note_constexpr_no_return : Note<
-  "control reached end of constexpr function">;
+  "constexpr function does not return a value">;
 def note_constexpr_virtual_call : Note<

I think this should remain past-tense, since it's describing a situation that 
actually //did// happen during constexpr evaluation. That is, "constexpr 
function did not return a value" or "control fell off the end of this function" 
or some such.


Repository:
  rC Clang

https://reviews.llvm.org/D45643



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

[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

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



Comment at: bindings/python/tests/cindex/test_diagnostics.py:18
 self.assertEqual(tu.diagnostics[0].spelling,
-'control reaches end of non-void function')
+'non-void function does not return a value')
 

It seems like part of the problem here is that "non-void function" is sort-of 
nonsense due to a few missing words. How about:

> "control can reach end of function with non-void return type"

or similar?

I think we still need the mention of control flow, because we're *not* saying 
the function contains no return statements, we're saying there's a control flow 
path that reaches the end of the function.



Comment at: docs/DiagnosticsReference.rst:9097
 |   |++|   
  |
 
+---+--+-+

This is a generated file; please don't manually update it. (Though we should 
regenerate it, it's probably quite stale...)


Repository:
  rC Clang

https://reviews.llvm.org/D45643



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

[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

2018-04-13 Thread easyaspi314 (Devin) via Phabricator via cfe-commits 
easyaspi314 created this revision.
easyaspi314 added a project: clang.
Herald added a subscriber: cfe-commits.

Replace [-Wreturn-type] messages, "control reaches/may reach end of non-void 
x", to "non-void x does/might not return a value".

F5962917: Screen Shot 2018-04-13 at 6.56.17 PM.png 


That warning is a very cryptic error copied from GCC, and I had to memorize the 
real meaning of the message in order to make sense of it.

If you were to merge this, 'Clang :: Misc/serialized-diags-stable.c' will fail.

I need some help on this one, as 
clang/test/Misc/Inputs/serialized-diags-stable.dia is a binary which needs to 
be regenerated, and I don't exactly know how to do that.


Repository:
  rC Clang

https://reviews.llvm.org/D45643

Files:
  bindings/python/tests/cindex/test_diagnostics.py
  docs/DiagnosticsReference.rst
  include/clang/Basic/DiagnosticASTKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Analysis/const-method-call.cpp
  test/Analysis/logical-ops.c
  test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m
  test/CXX/expr/expr.prim/expr.prim.lambda/p5.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p7.cpp
  test/CodeGenObjCXX/property-dot-reference.mm
  test/Driver/cc-log-diagnostics.c
  test/Frontend/absolute-paths.c
  test/Frontend/ast-main.cpp
  test/Index/warning-flags.c
  test/Misc/serialized-diags-stable.c
  test/Modules/redecl-merge.m
  test/PCH/late-parsed-instantiations.cpp
  test/Sema/block-return-1.c
  test/Sema/block-return-3.c
  test/Sema/freemain.c
  test/Sema/return.c
  test/SemaCXX/attr-noreturn.cpp
  test/SemaCXX/constant-expression-cxx1y.cpp
  test/SemaCXX/coreturn.cpp
  test/SemaCXX/cxx1y-deduced-return-type.cpp
  test/SemaCXX/return-noreturn.cpp
  test/SemaCXX/warn-missing-noreturn.cpp
  test/SemaTemplate/late-parsing-eager-instantiation.cpp

Index: test/SemaTemplate/late-parsing-eager-instantiation.cpp
===
--- test/SemaTemplate/late-parsing-eager-instantiation.cpp
+++ test/SemaTemplate/late-parsing-eager-instantiation.cpp
@@ -15,8 +15,8 @@
   char data() {
 visit([](auto buffer) -> char { // expected-note {{in instantiation}}
   buffer->data();
-}); // expected-warning {{control reaches end of non-void lambda}}
-  } // expected-warning {{control reaches end of non-void function}}
+}); // expected-warning {{non-void lambda does not return a value}}
+  } // expected-warning {{non-void function does not return a value}}
 };
 
 // pr34185
Index: test/SemaCXX/warn-missing-noreturn.cpp
===
--- test/SemaCXX/warn-missing-noreturn.cpp
+++ test/SemaCXX/warn-missing-noreturn.cpp
@@ -91,7 +91,7 @@
 
 int rdar8875247_test() {
   rdar8875247 f;
-} // expected-warning{{control reaches end of non-void function}}
+} // expected-warning{{non-void function does not return a value}}
 
 struct rdar8875247_B {
   rdar8875247_B();
Index: test/SemaCXX/return-noreturn.cpp
===
--- test/SemaCXX/return-noreturn.cpp
+++ test/SemaCXX/return-noreturn.cpp
@@ -26,7 +26,7 @@
   }
   int f2_positive(int x) {
 switch (x) { default: ; }
-  } // expected-warning {{control reaches end of non-void function}}
+  } // expected-warning {{non-void function does not return a value}}
   int f3(int x) {
 switch (x) { default: { pr6884_abort_struct(); } }
   }
@@ -46,7 +46,7 @@
   pr6884_abort_struct *p = new pr6884_abort_struct();
   delete p;
 }
-  } // expected-warning {{control reaches end of non-void function}}
+  } // expected-warning {{non-void function does not return a value}}
 
   // Test that these constructs work even when extraneous blocks are created
   // before and after the switch due to implicit destructors.
@@ -61,7 +61,7 @@
   int g2_positive(int x) {
 other o;
 switch (x) { default: ; }
-  } // expected-warning {{control reaches end of non-void function}}
+  } // expected-warning {{non-void function does not return a value}}
   int g3(int x) {
 other o;
 switch (x) { default: { pr6884_abort_struct(); } }
@@ -140,7 +140,7 @@
 default:
 break;
   }
-} // expected-warning {{control reaches end of non-void function}}
+} // expected-warning {{non-void function does not return a value}}
 
 void PR9412_f() {
 PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
@@ -165,7 +165,7 @@
 
 int testTernaryStaticallyConditionalRetrunOnTrue() {
   true ? Return() : NoReturn();
-} // expected-warning {{control reaches end of non-void function}}
+} // expected-warning {{non-void function does not return a value}}
 
 int testTernaryStaticallyConditionalNoretrunOnFalse() {
   false ? Return() : NoReturn();
@@ -173,23 +173,23 @@
 
 int testTernaryStaticallyConditionalRetrunOnFalse() {
   false ? NoReturn() : Return();
-} // expected-warning {{control