PiotrZSL added a comment.

@isuckatcs
No it were not fine, check function were executed, ExceptionAnalyzer created 
only to bail out due to nullptr getBody for functions with only declaration.
For functions with separate declaration and definition entire analysis is 
executed twice. simply because FunctionDecl::getBody jumps to function 
definition.
And this simply isn't needed, as we analyse definition anyway, so lets just 
emit warning for definition.
Now it forces developer to put NOLINT's into both .cpp and .hpp for same issue.

"The warning was emitted at every occurence of the function. It might be 
confusing if it's only emitted for the definition."
Why ? Issue is in definition, not declaration.

Example with indirectly_recursive & recursion_helper behave in same way, only 
difference is that warning is emitted only for definition.
"We know that indirectly_recursive(int n) throws when it shouldn't and that 
means recursion_helper(int n) will also throw when it shouldn't either."
Well no indirectly_recursive throws when it shouldn't but only because 
`thrower` throws, recursion_helper does not throw, it crashes. This is other 
bug that I'm fixing (not under this patch) related to properly handling 
noexcept keyword.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148462/new/

https://reviews.llvm.org/D148462

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

Reply via email to