aaron.ballman added a comment.
In https://reviews.llvm.org/D43120#1005100, @Szelethus wrote:
> I also came up with this problem:
>
> RegularException funcReturningExceptioniTest(int i) {
> return RegularException();
> }
>
> void returnedValueTest() {
> funcReturningExceptioniTest(3); //Should this emit a warning?
> }
>
>
> I'm not sure whether it'd be a good idea to warn about these cases. Unused
> return values can be found by many other means, and I'm afraid the standard
> library is filled with these cases.
I think it's fine to not warn on this. It can be caught by other means, but if
those turn out to be insufficient for some reason, we can address them in a
follow-up patch.
================
Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:45
+ diag(TemporaryExpr->getLocStart(),
+ "exception instantiated but not bound (did you intend to 'throw'?)");
+}
----------------
Szelethus wrote:
> aaron.ballman wrote:
> > I'm not keen on the wording here ("bound" isn't a common phrase for
> > exceptions). How about `"suspicious exception object created but not
> > thrown; did you mean 'throw %0'"` and then pass in the text for the object
> > construction?
> I tried using the `Lexer::getSourceText` function, but it doesn't print the
> right parantheses (ex. `RegularException(` instead of `RegularException()`).
>
> Also, are you sure it'd be a good idea to pass the entire statement there? In
> cases where it would be long, it could draw the programmer's attention away
> from the actual problem, the missing `throw` keyword.
That's a reasonable point, I'm fine with the way things are in this patch now.
================
Comment at: docs/ReleaseNotes.rst:70
+
+ Warns if a ``throw`` keyword is potentialy missing before a temporary
exception object.
+
----------------
This wording is a bit unclear. How about:
Diagnoses when a temporary object that appears to be an exception is
constructed but not thrown.
================
Comment at: test/clang-tidy/misc-throw-keyword-missing.cpp:33
+
+} //namespace std
+
----------------
Missing space after the comment introducer. This happens in other places as
well -- you should run the patch through clang-format to be sure to catch all
the formatting issues.
================
Comment at: test/clang-tidy/misc-throw-keyword-missing.cpp:54
+ if (i > 0)
+ //CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception object
created but not thrown; did you mean 'throw {{.*}}'?
[misc-throw-keyword-missing]
+ std::runtime_error("Unexpected argument");
----------------
Starting with this warning message and each subsequent message, you can drop
most of the wording. e.g., `:[[@LINE+1]]:5: warning: suspicious exception` is
sufficient.
================
Comment at: test/clang-tidy/misc-throw-keyword-missing.cpp:132
+
+RegularException funcReturningExceptioniTest(int i) {
+ return RegularException();
----------------
typo: Exceptioni
https://reviews.llvm.org/D43120
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
