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

[Torbjörn Klatt via cfe-dev]

Hi Don,

thank you very much for your advice.

After playing around a little time with this I finally went with your 
suggestion removing this branch altogether. No tests seem to fail.

I opened D61827 with the change.

Best
Torbjörn

On Saturday, May 11, 2019 6:08:31 AM CEST Don Hinton wrote:
> 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