[cfe-dev] clang-tidy FixItHint::CreateRemoval -- trailing semicolon stays

Piotr Dziwinski via cfe-dev cfe-dev at lists.llvm.org
Thu Feb 4 08:37:19 PST 2016


I ran into similar problem with semicolons a while ago.

I ended up doing something similar to your code snippet, but I 
copy-pasted some existing code from a different part of clang codebase:
lib/ARCMigrate/Transforms.cpp - see funcions findSemiAfterLocation() and 
findLocationAfterSemi().

Perhaps it would be a good idea to expose these functions as part of 
public API.

 > 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.

Yes, you can add many fix it hints for a given diagnostic. For example:

   auto Diagnostic = diag(Loc, "diagnostic");
   Diagnostic.AddFixItHint(FixItHint::CreateInsertion(/*...*/));
   Diagnostic.AddFixItHint(FixItHint::CreateRemoval(/*...*/));

This can also be achieved through overloaded operator<< which is just a 
wrapper for the same function calls.

Best regards,
Piotr Dziwinski

On 04/02/16 14:47, Tim Halloran via cfe-dev wrote:
> 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).
>
> 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.
>
> Thanks again, Tim Halloran
>
> My draft code to deal with ";"
>
> bool UsingCheck::inHeader(clang::SourceLocation &Loc,
>                           const clang::SourceManager *sm) {
>   return sm->getMainFileID() != sm->getFileID(Loc);
> }
>
> void UsingCheck::check(
>     const clang::ast_matchers::MatchFinder::MatchResult &Result) {
>   const UsingDirectiveDecl *U =
> Result.Nodes.getNodeAs<UsingDirectiveDecl>("using");
>   if (U) {
>     clang::SourceLocation Loc{U->getLocStart()};
>     if (U->isImplicit() || !Loc.isValid() ||
>         inHeader(Loc, Result.SourceManager))
>       return;
>
>     clang::SourceRange Range{U->getSourceRange()};
>
>     // There is a problem with the returned range -- it misses the ";".
>     // For example, for "using namespace std;" we don't get the 
> trailing ";".
>     // So the code below searches for a semicolon starting at the last 
> token
>     // of the node and expands the sourcerange so that the entire 
> statement
>     // may be removed. We should always find a semicolon.
>     int SemiIndex = 1;
>     bool foundSemi = false;
>     while (!foundSemi) {
>       clang::SourceLocation PastEndLoc{
> U->getLocEnd().getLocWithOffset(SemiIndex)};
>       clang::SourceRange RangeForString{PastEndLoc};
>       CharSourceRange CSR = Lexer::makeFileCharRange(
> CharSourceRange::getTokenRange(RangeForString), *Result.SourceManager,
>           Result.Context->getLangOpts());
>       std::string possibleSemi =
>           Lexer::getSourceText(CSR, *Result.SourceManager,
>  Result.Context->getLangOpts())
>               .str();
>       if (possibleSemi == ";") {
>         // Found a ";" so expand the range for our fixit below.
>         Range.setEnd(PastEndLoc);
>         foundSemi = true;
>       } else {
>         SemiIndex++;
>       }
>     }
>
>     diag(Loc, "do not use namespace using-directives; "
>               "use using-declarations instead")
>         << FixItHint::CreateRemoval(Range);
>   }
> <<Snip lots more code that is in the works>>
>
>
>
> On Thu, Feb 4, 2016 at 8:22 AM, Manuel Klimek <klimek at google.com 
> <mailto:klimek at google.com>> wrote:
>
>     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).
>
>     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...
>
>     On Tue, Feb 2, 2016 at 7:31 PM Tim Halloran via cfe-dev
>     <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>
>         Hopefully a simple question. I'm removing a using namespace
>         directive from the code in a simple check. so
>
>         using namespace std;
>
>         gets removed from a C++ file my code however ends up with
>
>         ;
>
>         when I run
>
>         clang-tidy -checks=-*,surelogic* -fix-errors test1.cpp --
>         -std=c++14
>         4 warnings generated.
>         /home/tim/Source/llvm-work/test1.cpp:8:1: warning: do not use
>         namespace using-directives; use using-declarations instead
>         [surelogic-readability-using]
>         using namespace std;
>         ^
>         /home/tim/Source/llvm-work/test1.cpp:8:1: note: FIX-IT applied
>         suggested code changes
>         using namespace std;
>         ^
>         <<snip>>
>         clang-tidy applied 2 of 2 suggested fixes.
>
>         The non-boilerplate code is below:
>
>          void UsingCheck::registerMatchers(ast_matchers::MatchFinder
>         *Finder) {
>             // Only register the matchers for C++; the functionality
>         currently does not
>             // provide any benefit to other languages, despite being
>         benign.
>             if (getLangOpts().CPlusPlus)
>         Finder->addMatcher(usingDirectiveDecl().bind("usingNamespace"), this);
>           }
>           void
>         UsingCheck::check(const MatchFinder::MatchResult &Result) {
>           const auto *U =
>         Result.Nodes.getNodeAs<UsingDirectiveDecl>("usingNamespace");
>           SourceLocation Loc = U->getLocStart();
>             if (U->isImplicit() || !Loc.isValid())
>               return;
>             diag(Loc, "do not use namespace using-directives; "
>                     "use using-declarations instead") <<
>         FixItHint::CreateRemoval(U->getSourceRange());
>           }
>
>         (yes this is clone of the google check, just to learn)
>
>         But I'm wondering what to do to get the semicolon. I don't see
>         it in the AST dump output...below
>
>         <<snip stuff in my test code>>
>         |-FunctionDecl 0x482c160 <mod.cpp:5:1, col:37> col:5 used foo
>         'int (class Point)'
>         | |-ParmVarDecl 0x482c098 <col:9, col:15> col:15 used p 'class
>         Point'
>         | `-CompoundStmt 0x482c2b0 <col:18, col:37>
>         |   `-ReturnStmt 0x482c298 <col:20, col:34>
>         |     `-CXXMemberCallExpr 0x482c270 <col:27, col:34> 'int'
>         |       `-MemberExpr 0x482c238 <col:27, col:29> '<bound member
>         function type>' .getX 0x482b178
>         |         `-DeclRefExpr 0x482c210 <col:27> 'class Point'
>         lvalue ParmVar 0x482c098 'p' 'class Point'
>         << The using directive is next >>
>         |-UsingDirectiveDecl 0x482c2d0 <line:8:1, col:17> col:17
>         Namespace 0x409b938 'std'
>         << next a function decl in my test code>>
>         `-FunctionDecl 0x482c340 <line:10:1, line:41:1> line:10:5 main
>         'int (void)'
>           `-CompoundStmt 0x486d200 <col:12, line:41:1>
>
>         Any suggestions?
>         -- 
>         Tim Halloran
>         SureLogic, Inc.
>         5808 Forbes Avenue, Pittsburgh PA 15217-1602
>         (412) 722-3338 <tel:%28412%29%20722-3338>
>         _______________________________________________
>         cfe-dev mailing list
>         cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
>         http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
>
> -- 
> Tim Halloran
> SureLogic, Inc.
> 5808 Forbes Avenue, Pittsburgh PA 15217-1602
> (412) 722-3338
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://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/20160204/63490b57/attachment.html>


More information about the cfe-dev mailing list