[cfe-dev] [clang-tidy] problem finding AST matcher for nested implicit const-cast

[Don Hinton via cfe-dev]

I think you might be able to safely remove this `if else`, or at least
limit it.

Consider the following:

// this is your test case. it's legal, but currently ignored
std::vector<int> v;
for(std::vector<int>::const_iterator i = v.begin(); i != v.end(); ++i) {}

// these aren't illegal, and therefore never even considered by the checker
-- they cause a compiler error
for(std::vector<int>::iterator i = v.cbegin(); i != v.cend(); ++i) {}
const std::vector<int> cv;
for(std::vector<int>::iterator i = cv.begin(); i != cv.end(); ++i) {}




On Fri, May 10, 2019 at 4:45 PM Don Hinton <hintonda at gmail.com> wrote:

> Hi Torbjörn:
>
> This behavior appears to be intentional, though it would be friendly to
> emit a warning.  I believe this is the case your are hitting:
>
>    794      } else if (!Context->hasSameType(CanonicalInitVarType,
>    795                                       CanonicalBeginType)) {
>    796        // Check for qualified types to avoid conversions from
> non-const to const
>    797        // iterator types.
>    798        return false;
>    799      }
>
> hth...
> don
>
> On Tue, May 7, 2019 at 1:06 PM Torbjörn Klatt via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> Hi all,
>>
>> I'm trying to fix the clang-tidy bug 35082 [1] as a starter for getting
>> into LLVM development and contribution.
>>
>> I think, I've tracked down the issue to
>>
>> clang-tidy/modernize/LoopConvertCheck.cpp:123ff:
>> clang::tidy::modernize::makeIteratorLoopMatcher()
>> ```
>>   DeclarationMatcher InitDeclMatcher =
>>
>> varDecl(hasInitializer(anyOf(ignoringParenImpCasts(BeginCallMatcher),
>>                                    materializeTemporaryExpr(
>>
>>  ignoringParenImpCasts(BeginCallMatcher)),
>>                                    hasDescendant(BeginCallMatcher))))
>>           .bind(InitVarName);
>> ```
>> The definition of `InitDeclMatcher` seems to not match  the AST tree
>> ```
>>   |-ForStmt
>>   | |-DeclStmt
>>   | | `-VarDecl
>>   | |   `-ExprWithCleanups
>>   | |     `-ImplicitCastExpr
>>   | |       `-CXXConstructExpr
>>   | |         `-MaterializeTemporaryExpr
>>   | |           `-ImplicitCastExpr
>>   | |             `-CXXMemberCallExpr
>>   | |               `-MemberExpr
>>   | |                 `-DeclRefExpr
>> ```
>> of
>> ```
>> std::vector<int> vec{1, 2, 3, 4, 5};
>> for(std::vector<int>::const_iterator i = vec.begin(); i != vec.end();
>> ++i) { ... }
>> ```
>>
>> In my naivety, I tried to include the following into the `anyOf`:
>> ```
>>                                    exprWithCleanups(
>>                                        ignoringParenImpCasts(
>>                                            cxxConstructExpr(
>>                                                materializeTemporaryExpr(
>>                                                    ignoringParenImpCasts(
>>
>>  BeginCallMatcher))))),
>> ```
>> mimicking the specific AST tree. Without luck.
>>
>> I browsed through include/clang/ASTMatchers.h for better suited matchers,
>> but couldn't find any.
>>
>> What important detail am I missing?
>>
>> Best
>> Torbjörn
>>
>> [1]: https://bugs.llvm.org/show_bug.cgi?id=35082
>>
>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190510/0dd82aa7/attachment.html>