koldaniel added inline comments.

================
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:66-67
+    }
+    Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+                                       "s");
+  }
----------------
aaron.ballman wrote:
> Same concerns here as with the previous review: This fixit can break code if 
> the code disallows narrowing conversions. e.g.,
> ```
> bool b{std::uncaught_exception()};
> ```
> In this case, the fixit will take the deprecated code and make it ill-formed 
> instead. Perhaps a better fix-it would be to transform the code into 
> (std::uncaught_exceptions() > 0) to keep the resulting expression type a bool 
> and still not impact operator precedence. Alternatively, you can identify the 
> narrowing conversion case and skip the fix-it entirely in that case (only 
> warn).
> 
> This example should be a test case.
If the fix-it would be a transformation to std::uncaught_exceptions() > 0, the 
code will break in some cases, for example in case of function pointers or 
template instantiations. Narrowing conversions would not lead to errors, 
functionality of the code would remain the same.


https://reviews.llvm.org/D40787



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

Reply via email to