[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 16 13:10:41 PST 2023


xazax.hun added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:81
+          FromPointeeTy->getUnqualifiedDesugaredType();
+      const Type *ToPointeeUQTy = ToPointeeTy->getUnqualifiedDesugaredType();
+
----------------
Here I am also wondering if we actually want canonical types.


================
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:95
+
+  if (P1->isArrayType() && P2->isArrayType()) {
+    // At every level that array type is involved in, at least
----------------
isuckatcs wrote:
> xazax.hun wrote:
> > isuckatcs wrote:
> > > xazax.hun wrote:
> > > > If none of the functions I recommended works for you, I still strongly suggest reusing utilities from ASTContext, like `UnwrapSimilarTypes` and `UnwrapSimilarArrayTypes`.
> > > I didn't check all of the functions inside `ASTContext`, but here we have very specific rules, we need to check. One utility might help with one check or two, but won't replace the need to go ove all of them. Also I think it's easier to understand which section checks what, if I keep them separated.
> > > 
> > > In an ealier comment I link to [[ https://en.cppreference.com/w/cpp/language/implicit_conversion#Qualification_conversions | the section on cppreference ]], which discusses what these rules are. Also there I found one controversial example, that's only detected by MSVC. I wonder if you know why that happens.
> > I see. It is unfortunate that we cannot simply reuse the corresponding functionality from Sema. The code mostly looks good to me, apart from some nits.
> > It is unfortunate that we cannot simply reuse the corresponding functionality from Sema.
> 
> It is indeed unfortunate, though I wonder if we want to move this functionality to `ASTContext`, so that it can be reused outside the `ExceptionAnalyzer`.
I am not opposed to that, but I think in that case we want this method to be used in the regular compilation pipeline to make sure it is correct. 


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

https://reviews.llvm.org/D135495



More information about the cfe-commits mailing list