[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

Mon Jan 28 05:03:01 PST 2019

JonasToth added a comment.

In D57100#1373430 <https://reviews.llvm.org/D57100#1373430>, @baloghadamsoftware wrote:

> Hello! I am glad to see that this check gets improved by the community. I also think a "modernize" check which marks functions with `noexcept` is also useful.

Great to hear!

> As for the comma or semicolon, I think semicolon would be better. I see that this part is reverted now, but is there any reason for using `std::string` instead of `llvm::StringRef`? I always thought this latter is the standard for LLVM code wherever possible.

Which part to you mean? The original semicolon-splitting?
I think there the issue is, that `StringRef` is just a handle, but if you split a big string into smaller ones you want the result to be owning its own memory instead of just pointing to a span in the original full string.
Even though that would be correct in some cases it has a subtle dangling-bug if you work further with the result of the split or the underlying string disappears for some reason.
Otherwise `StringRef` is definitly the goto-class for string operations (that dont change underlying memory).

For now the comma-vs-semicolon thingie will stay as is, because its a breaking change in the configurations and the gain for switching to semicolon is small, whereas the pain to figuring out that changed is big.

@baloghadamsoftware Do you see any problems with the refactoring, especially the new `FunctionDecl*`-Caching is not exactly a refactoring. If you have any thoughts on that would be great to hear!

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57100