<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Tim,<div class=""><br class=""></div><div class="">Iterating through the SourceLocations is a nice solution, but I think the check might accidentally catch the semicolon in the comment IIUC.</div><div class=""><br class=""></div><div class="">Example:</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>using namespace std /* ; */ ;</div><div class=""><br class=""></div><div class="">To avoid that you might want to get tok::TokenKind and compare it against tok::semi, which corresponds to the semicolon. To see how this works, refer to clang-tidy readability-braces-around-statements check. It kind of deals with the same “problem”.</div><div class=""><br class=""></div><div class="">Though I’m dealing with the same at the moment, so I guess it’d be nice to get a “universal solution” and put it somewhere (in clang-tidy?) at some point, because some clang-tidy checks are going through that and having a common solution would reduce the amount of possible bugs caused by different implementations etc.</div><div class=""><br class=""></div><div class="">—</div><div class="">Kirill Bobyrev</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 04 Feb 2016, at 5:47 PM, Tim Halloran via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">Thanks again, Tim Halloran</div><div class=""><br class=""></div><div class="">My draft code to deal with ";"<br class=""><div class=""><br class=""></div><div class=""><div class="">bool UsingCheck::inHeader(clang::SourceLocation &Loc,</div><div class=""> const clang::SourceManager *sm) {</div><div class=""> return sm->getMainFileID() != sm->getFileID(Loc);</div><div class="">}</div><div class=""><br class=""></div><div class="">void UsingCheck::check(</div><div class=""> const clang::ast_matchers::MatchFinder::MatchResult &Result) {</div><div class=""> const UsingDirectiveDecl *U =</div><div class=""> Result.Nodes.getNodeAs<UsingDirectiveDecl>("using");</div><div class=""> if (U) {</div><div class=""> clang::SourceLocation Loc{U->getLocStart()};</div><div class=""> if (U->isImplicit() || !Loc.isValid() ||</div><div class=""> inHeader(Loc, Result.SourceManager))</div><div class=""> return;</div><div class=""><br class=""></div><div class=""> clang::SourceRange Range{U->getSourceRange()};</div><div class=""><br class=""></div><div class=""> // There is a problem with the returned range -- it misses the ";".</div><div class=""> // For example, for "using namespace std;" we don't get the trailing ";".</div><div class=""> // So the code below searches for a semicolon starting at the last token</div><div class=""> // of the node and expands the sourcerange so that the entire statement</div><div class=""> // may be removed. We should always find a semicolon.</div><div class=""> int SemiIndex = 1;</div><div class=""> bool foundSemi = false;</div><div class=""> while (!foundSemi) {</div><div class=""> clang::SourceLocation PastEndLoc{</div><div class=""> U->getLocEnd().getLocWithOffset(SemiIndex)};</div><div class=""> clang::SourceRange RangeForString{PastEndLoc};</div><div class=""> CharSourceRange CSR = Lexer::makeFileCharRange(</div><div class=""> CharSourceRange::getTokenRange(RangeForString), *Result.SourceManager,</div><div class=""> Result.Context->getLangOpts());</div><div class=""> std::string possibleSemi =</div><div class=""> Lexer::getSourceText(CSR, *Result.SourceManager,</div><div class=""> Result.Context->getLangOpts())</div><div class=""> .str();</div><div class=""> if (possibleSemi == ";") {</div><div class=""> // Found a ";" so expand the range for our fixit below.</div><div class=""> Range.setEnd(PastEndLoc);</div><div class=""> foundSemi = true;</div><div class=""> } else {</div><div class=""> SemiIndex++;</div><div class=""> }</div><div class=""> }</div><div class=""><br class=""></div><div class=""> diag(Loc, "do not use namespace using-directives; "</div><div class=""> "use using-declarations instead")</div><div class=""> << FixItHint::CreateRemoval(Range);</div><div class=""> }</div><div class=""><<Snip lots more code that is in the works>></div><div class=""><br class=""></div><div class=""><br class=""></div></div></div></div><div class="gmail_extra" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""><div class="gmail_quote">On Thu, Feb 4, 2016 at 8:22 AM, Manuel Klimek<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:klimek@google.com" target="_blank" class="">klimek@google.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><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" class=""><div class="gmail_quote"><div class="">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 class=""><br class=""></div><div class="">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 class=""><div class="h5"><div dir="ltr" class=""><br class=""></div><div dir="ltr" class="">On Tue, Feb 2, 2016 at 7:31 PM Tim Halloran via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" class="">cfe-dev@lists.llvm.org</a>> wrote:<br class=""></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 class=""><div class="h5"><div dir="ltr" class="">Hopefully a simple question. I'm removing a using namespace directive from the code in a simple check. so<div class=""><br class=""></div><div class="">using namespace std;</div><div class=""><br class=""></div><div class="">gets removed from a C++ file my code however ends up with</div><div class=""><br class=""></div><div class="">;</div><div class=""><br class=""></div><div class="">when I run</div><div class=""><br class=""></div><div class=""><div class="">clang-tidy -checks=-*,surelogic* -fix-errors test1.cpp -- -std=c++14</div><div class="">4 warnings generated.</div><div class="">/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 class="">using namespace std;</div><div class="">^</div><div class="">/home/tim/Source/llvm-work/test1.cpp:8:1: note: FIX-IT applied suggested code changes</div><div class="">using namespace std;</div><div class="">^</div><div class=""><<snip>></div><div class="">clang-tidy applied 2 of 2 suggested fixes.<br class=""></div><div class=""><br class=""></div><div class="">The non-boilerplate code is below:</div><div class=""><br class=""></div><div class=""><div class=""> void UsingCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {</div><div class=""> <span class="Apple-converted-space"> </span>// Only register the matchers for C++; the functionality currently does not</div><div class=""> <span class="Apple-converted-space"> </span>// provide any benefit to other languages, despite being benign.</div><div class=""> <span class="Apple-converted-space"> </span>if (getLangOpts().CPlusPlus)</div><div class=""> <span class="Apple-converted-space"> </span>Finder->addMatcher(usingDirectiveDecl().bind("usingNamespace"), this);</div><div class=""> <span class="Apple-converted-space"> </span>}</div><div class=""> </div><div class=""> <span class="Apple-converted-space"> </span>void</div><div class="">UsingCheck::check(const MatchFinder::MatchResult &Result) {</div><div class=""> const auto *U = Result.Nodes.getNodeAs<UsingDirectiveDecl>("usingNamespace");</div><div class=""> <span class="Apple-converted-space"> </span>SourceLocation Loc = U->getLocStart();</div><div class=""> <span class="Apple-converted-space"> </span>if (U->isImplicit() || !Loc.isValid())</div><div class=""> <span class="Apple-converted-space"> </span>return;</div><div class=""> </div><div class=""> <span class="Apple-converted-space"> </span>diag(Loc, "do not use namespace using-directives; "</div><div class=""> <span class="Apple-converted-space"> </span>"use using-declarations instead") << FixItHint::CreateRemoval(U->getSourceRange());</div><div class=""> <span class="Apple-converted-space"> </span>}</div></div><div class=""><br class=""></div><div class=""><div class="">(yes this is clone of the google check, just to learn)</div><div class=""><br class=""></div><div class="">But I'm wondering what to do to get the semicolon. I don't see it in the AST dump output...below</div><div class=""><br class=""></div><div class=""><<snip stuff in my test code>></div><div class=""><div class="">|-FunctionDecl 0x482c160 <mod.cpp:5:1, col:37> col:5 used foo 'int (class Point)'</div><div class="">| |-ParmVarDecl 0x482c098 <col:9, col:15> col:15 used p 'class Point'</div><div class="">| `-CompoundStmt 0x482c2b0 <col:18, col:37></div><div class="">| `-ReturnStmt 0x482c298 <col:20, col:34></div><div class="">| `-CXXMemberCallExpr 0x482c270 <col:27, col:34> 'int'</div><div class="">| `-MemberExpr 0x482c238 <col:27, col:29> '<bound member function type>' .getX 0x482b178</div><div class="">| `-DeclRefExpr 0x482c210 <col:27> 'class Point' lvalue ParmVar 0x482c098 'p' 'class Point'</div><div class=""><< The using directive is next >></div><div class="">|-UsingDirectiveDecl 0x482c2d0 <line:8:1, col:17> col:17 Namespace 0x409b938 'std'</div><div class=""><< next a function decl in my test code>></div><div class="">`-FunctionDecl 0x482c340 <line:10:1, line:41:1> line:10:5 main 'int (void)'</div><div class=""> <span class="Apple-converted-space"> </span>`-CompoundStmt 0x486d200 <col:12, line:41:1></div></div><div class=""><br class=""></div><div class="">Any suggestions?</div>--<span class="Apple-converted-space"> </span><br class=""><div class="">Tim Halloran<br class="">SureLogic, Inc.<br class="">5808 Forbes Avenue, Pittsburgh PA 15217-1602<br class=""><a href="tel:%28412%29%20722-3338" value="+14127223338" target="_blank" class="">(412) 722-3338</a></div></div></div></div></div></div>_______________________________________________<br class="">cfe-dev mailing list<br class=""><a href="mailto:cfe-dev@lists.llvm.org" target="_blank" class="">cfe-dev@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br class=""></blockquote></div></div></blockquote></div><br class=""><br clear="all" class=""><div class=""><br class=""></div>--<span class="Apple-converted-space"> </span><br class=""><div class="gmail_signature">Tim Halloran<br class="">SureLogic, Inc.<br class="">5808 Forbes Avenue, Pittsburgh PA 15217-1602<br class="">(412) 722-3338</div></div><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">cfe-dev mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="mailto:cfe-dev@lists.llvm.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">cfe-dev@lists.llvm.org</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a></div></blockquote></div><br class=""></div></body></html>