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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
