[PATCH] D148461: [clang-tidy] Support C++17/20 in bugprone-exception-escape

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:306
+
+      if (From->isMemberPointerType() || To->isMemberPointerType())
         return false;
----------------
isuckatcs wrote:
> PiotrZSL wrote:
> > isuckatcs wrote:
> > > isuckatcs wrote:
> > > > Please cover this line with both positive and negative test cases.
> > > > 
> > > > Also upon looking up both [[ https://www.open-std.org/jtc1/sc22/wg21/docs/standards | N4849 ]] (C++ 20 draft) and [[ https://github.com/cplusplus/draft/releases | N4917]] (C++ 23 draft), they both say for qualification conversion that 
> > > > 
> > > > 
> > > > > each P_i is ... pointer to member of class C_i of type, ...
> > > > 
> > > > Why are we not allowing them if the standard is at least C++ 20?
> > > Please resolve this thread.
> > Positive case is tested by throw_basefn_catch_derivedfn test and throw_derivedfn_catch_basefn.
> > Negative is covered by throw_basefn_catch_basefn.
> > 
> > For members there are tests throw_basem_catch_basem, throw_basem_catch_derivedm, throw_derivedm_catch_basem that also covers this correctly.
> > 
> > Tested this with GCC, and behavior is proper and independent to C++ standard.
> > Tested this with GCC, and behavior is proper and independent to C++ standard.
> 
> I don't know how to deal with this tbh. In a static analyzer we usually want to consider what the standard says and not what a specific compiler implementation does, as the compiler can still be buggy, lack the proper support for the standard, etc.
> 
> Maybe we can add a FIXME here that explains, why this check is here and that it's the opposite of what the standard says. Then later if it causes issues, it will be easy to see why that happens.
> 
> @xazax.hun do you have a prefered method to deal with these situations in the analyzer? If there is one, we could also try it here.
@PiotrZSL this is still unanswered. The standard says these are allowed, so according to the standard we model something wrong here, unless I'm misunderstanding something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148461