<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Feb 4, 2016 at 3:47 PM, Tim Halloran <span dir="ltr"><<a href="mailto:tim.halloran@surelogic.com" target="_blank">tim.halloran@surelogic.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Thanks, after looking into this your answer makes sense to me. What I ended up doing is shown in the snippet below. As you might guess this code is working to remove using namespace and provide fixes to all uses in the translation unit (thus it guards against any changes in an included file -- focusing just on the main translation unit).<div><br></div><div>Are there any good examples of applying multiple fixits for a problem (can I just stream many in?). Just wondering as I have not tried that out much yet.</div></div></blockquote><div><br></div><div>You can supply many fixes to the same warning as long as they don't overlap.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Thanks again, Tim Halloran</div><div><br></div><div>My draft code to deal with ";"<br><div><br></div><div><div>bool UsingCheck::inHeader(clang::SourceLocation &Loc,</div><div> const clang::SourceManager *sm) {</div><div> return sm->getMainFileID() != sm->getFileID(Loc);</div></div></div></div></blockquote><div><br></div><div>It's an incorrect way to detect headers, since the check can run on the header directly and then getMainFileID will correspond to the header. There's a set of utilities being added to ClangTidy (<a href="http://reviews.llvm.org/D16113">http://reviews.llvm.org/D16113</a>) to use filename extensions for this purpose.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div><div>}</div><div><br></div><div>void UsingCheck::check(</div><div> const clang::ast_matchers::MatchFinder::MatchResult &Result) {</div><div> const UsingDirectiveDecl *U =</div><div> Result.Nodes.getNodeAs<UsingDirectiveDecl>("using");</div><div> if (U) {</div><div> clang::SourceLocation Loc{U->getLocStart()};</div><div> if (U->isImplicit() || !Loc.isValid() ||</div><div> inHeader(Loc, Result.SourceManager))</div><div> return;</div><div><br></div><div> clang::SourceRange Range{U->getSourceRange()};</div><div><br></div><div> // There is a problem with the returned range -- it misses the ";".</div><div> // For example, for "using namespace std;" we don't get the trailing ";".</div><div> // So the code below searches for a semicolon starting at the last token</div><div> // of the node and expands the sourcerange so that the entire statement</div><div> // may be removed. We should always find a semicolon.</div><div> int SemiIndex = 1;</div><div> bool foundSemi = false;</div><div> while (!foundSemi) {</div><div> clang::SourceLocation PastEndLoc{</div><div> U->getLocEnd().getLocWithOffset(SemiIndex)};</div><div> clang::SourceRange RangeForString{PastEndLoc};</div><div> CharSourceRange CSR = Lexer::makeFileCharRange(</div><div> CharSourceRange::getTokenRange(RangeForString), *Result.SourceManager,</div><div> Result.Context->getLangOpts());</div><div> std::string possibleSemi =</div><div> Lexer::getSourceText(CSR, *Result.SourceManager,</div><div> Result.Context->getLangOpts())</div><div> .str();</div></div></div></div></blockquote><div><br></div><div>No need to convert this to std::string. There's actually a better way to find the next semicolon, see the usage of Lexer::getRawToken in NamespaceCommentCheck.cpp, for example. We should probably add something more convenient for this purpose to clang-tidy/utils/LexerUtils.h.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div><div> if (possibleSemi == ";") {</div><div> // Found a ";" so expand the range for our fixit below.</div><div> Range.setEnd(PastEndLoc);</div><div> foundSemi = true;</div><div> } else {</div><div> SemiIndex++;</div><span class=""><div> }</div><div> }</div><div><br></div><div> diag(Loc, "do not use namespace using-directives; "</div><div> "use using-declarations instead")</div></span><div> << FixItHint::CreateRemoval(Range);</div><div> }</div><div><<Snip lots more code that is in the works>></div><div><br></div><div><br></div></div></div></div><div class=""><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 4, 2016 at 8:22 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Unfortunately clang doesn't have the location of the semicolon in statements, mostly because Statements are the base class of Expressions (for historical reasons that do not apply any more).</div><div><br></div><div>Thus, fixits that want to get to the semicolon need to use the Lexer to get at it. Looping in clang-tidy folks for whether we have infrastructure in clang-tidy for that already...</div><div><div><div dir="ltr"><br></div><div dir="ltr">On Tue, Feb 2, 2016 at 7:31 PM Tim Halloran via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><div dir="ltr">Hopefully a simple question. I'm removing a using namespace directive from the code in a simple check. so<div><br></div><div>using namespace std;</div><div><br></div><div>gets removed from a C++ file my code however ends up with</div><div><br></div><div>;</div><div><br></div><div>when I run</div><div><br></div><div><div>clang-tidy -checks=-*,surelogic* -fix-errors test1.cpp -- -std=c++14</div><div>4 warnings generated.</div><div>/home/tim/Source/llvm-work/test1.cpp:8:1: warning: do not use namespace using-directives; use using-declarations instead [surelogic-readability-using]</div><div>using namespace std;</div><div>^</div><div>/home/tim/Source/llvm-work/test1.cpp:8:1: note: FIX-IT applied suggested code changes</div><div>using namespace std;</div><div>^</div><div><<snip>></div><div>clang-tidy applied 2 of 2 suggested fixes.<br></div><div><br></div><div>The non-boilerplate code is below:</div><div><br></div><div><div> void UsingCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {</div><div> // Only register the matchers for C++; the functionality currently does not</div><div> // provide any benefit to other languages, despite being benign.</div><div> if (getLangOpts().CPlusPlus)</div><div> Finder->addMatcher(usingDirectiveDecl().bind("usingNamespace"), this);</div><div> }</div><div> </div><div> void</div><div>UsingCheck::check(const MatchFinder::MatchResult &Result) {</div><div> const auto *U = Result.Nodes.getNodeAs<UsingDirectiveDecl>("usingNamespace");</div><div> SourceLocation Loc = U->getLocStart();</div><div> if (U->isImplicit() || !Loc.isValid())</div><div> return;</div><div> </div><div> diag(Loc, "do not use namespace using-directives; "</div><div> "use using-declarations instead") << FixItHint::CreateRemoval(U->getSourceRange());</div><div> }</div></div><div><br></div><div><div>(yes this is clone of the google check, just to learn)</div><div><br></div><div>But I'm wondering what to do to get the semicolon. I don't see it in the AST dump output...below</div><div><br></div><div><<snip stuff in my test code>></div><div><div>|-FunctionDecl 0x482c160 <mod.cpp:5:1, col:37> col:5 used foo 'int (class Point)'</div><div>| |-ParmVarDecl 0x482c098 <col:9, col:15> col:15 used p 'class Point'</div><div>| `-CompoundStmt 0x482c2b0 <col:18, col:37></div><div>| `-ReturnStmt 0x482c298 <col:20, col:34></div><div>| `-CXXMemberCallExpr 0x482c270 <col:27, col:34> 'int'</div><div>| `-MemberExpr 0x482c238 <col:27, col:29> '<bound member function type>' .getX 0x482b178</div><div>| `-DeclRefExpr 0x482c210 <col:27> 'class Point' lvalue ParmVar 0x482c098 'p' 'class Point'</div><div><< The using directive is next >></div><div>|-UsingDirectiveDecl 0x482c2d0 <line:8:1, col:17> col:17 Namespace 0x409b938 'std'</div><div><< next a function decl in my test code>></div><div>`-FunctionDecl 0x482c340 <line:10:1, line:41:1> line:10:5 main 'int (void)'</div><div> `-CompoundStmt 0x486d200 <col:12, line:41:1></div></div><div><br></div><div>Any suggestions?</div>-- <br><div>Tim Halloran<br>SureLogic, Inc.<br>5808 Forbes Avenue, Pittsburgh PA 15217-1602<br><a href="tel:%28412%29%20722-3338" value="+14127223338" target="_blank">(412) 722-3338</a></div>
</div></div></div></div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Tim Halloran<br>SureLogic, Inc.<br>5808 Forbes Avenue, Pittsburgh PA 15217-1602<br><a href="tel:%28412%29%20722-3338" value="+14127223338" target="_blank">(412) 722-3338</a></div>
</div>
</div></div></blockquote></div><br></div></div>