[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

This revision was automatically updated to reflect the committed changes. Closed by commit rGfc5f14c5fe78: [clang-tidy] Ignore declarations in bugprone-exception-escape (authored by PiotrZSL). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

isuckatcs accepted this revision. isuckatcs added a comment. This revision is now accepted and ready to land. I think there's no point of holding back this patch. Even though I'm not 100% sure we want this change, I say let's merge it and see how our users react. It's a one line change anyway,

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

carlosgalvezp added a comment. I would agree that the fact that a function throws or not can only be found out when analyzing the function definition (it's impossible to know from the declaration). There's also a duplicate diagnostic that I don't see much value on, so I would agree with this

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

PiotrZSL added a comment. @njames93 What do you thing ? Should bugprone-exception-escape provide warnings for all forward declarations and definition, or only for definition. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148462/new/

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

isuckatcs added a comment. > No because indirectly_recursive called from recursion_helper is noexcept, so > there will be std::terminate called. I missed that in `noexcept` functions thrown exceptions are not propagated. In this case I agree that `recursion_helper()` shouldn't emit a warning.

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

PiotrZSL added a comment. @isuckatcs "Note that this particular warning is reported for the function and not for something inside the definition." Function declaration is not a function. A function declaration is a statement in programming languages that declares the existence of a function,

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

PiotrZSL added a comment. @isuckatcs "Technically the exception is propagated through the function until a handler is found that catches it." No because indirectly_recursive called from recursion_helper is noexcept, so there will be std::terminate called. "Also we have cross translation unit

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

isuckatcs added a comment. > "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. For me it would be confusing, because the forward declaration is naming the same function

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

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

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

isuckatcs added a comment. I think the original behaviour was fine. The warning was emitted at every occurence of the function. It might be confusing if it's only emitted for the definition. Also what happens in the following scenario: int indirectly_recursive(int n) noexcept; int

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

PiotrZSL created this revision. PiotrZSL added reviewers: njames93, carlosgalvezp. Herald added a subscriber: xazax.hun. Herald added a project: All. PiotrZSL requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Warnings will now