[PATCH] D115118: [clang-tidy] Assume that `noexcept` functions won't throw anything in `bugprone-exception-escape` check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:201-202
+      case EST_BasicNoexcept:
+      case EST_DependentNoexcept: // Let's be optimistic here (necessary e.g.
+                                  // for variant assignment; see PR#52254)
+      case EST_NoexceptTrue:
----------------
Hmmm, I'm not certain I agree that we should be optimistic. If the exception specification is unknown and we want to be conservative, we can't determine that it might not throw, we can only determine that it might throw.

So this might be the correct behavior for this check but the incorrect behavior for other checks.

One possible approach is to not give an answer until the noexcept specification is resolved (e.g., return an `Optional<bool>` where `None` means "I can't answer the question", and callers have to react to that), another would be to pass in an argument specifying whether the caller wants to be conservative or aggressive in this case (that's how we handle "does this expression have side effects" in Clang).


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-exception-escape.cpp:311
+  est_noexcept_true();
+  est_dependent_noexcept<ShouldThrow<true>>();
+}
----------------
Do we still issue the diagnostic if you remove this instantiation?

Can you add a second instantiation: `est_dependent_noexcept<ShouldThrow<false>>();` to show that it does not diagnose?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115118