<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Feb 4, 2016 at 10:39 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@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_extra"><div class="gmail_quote"><span class="">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"><br></div></blockquote></span><span 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"><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></span><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" target="_blank">http://reviews.llvm.org/D16113</a>) to use filename extensions for this purpose.</div></div></div></div></blockquote><div><br></div><div>Thanks for the pointer, I looked this HeaderFileExtensionsUtils code over (on trunk) and it seems to examine filename suffix. It also seem configurable but not sure how (you set an option with "h, H" -- it would be nice to have a comment showing how to do this or documentation -- albeit I might not have found that yet).</div><div><br></div><div>However, that said, my check removes all included files, but, as you say, would not handle being run directly on a header file. However, my check seems necessary but not sufficient. If I changed it to use HeaderFileExtensionsUtils I don't see how includes like "string" or "iostream" would be excluded in my check?</div><div><br></div><div>To fix my code I probably should check that the extension of the main file is "cpp" or "cc" or "cxx" -- is there a utility for this? In face looking over HeaderFileExtensionsUtils I wonder why it isn't just FileExtensionsUtils -- it could consider any extensions really.</div><div><br></div><div>Thanks for the pointer!</div><div><br></div><div>Best,</div><div>Tim</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 class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><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></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><div class="h5"><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><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><div><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></div></div><br></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Tim Halloran<br>SureLogic, Inc.<br>5808 Forbes Avenue, Pittsburgh PA 15217-1602<br>(412) 722-3338</div>
</div></div>