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

Tim Halloran via cfe-dev cfe-dev at lists.llvm.org
Thu Feb 4 06:47:16 PST 2016


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> 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> 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
>> _______________________________________________
>> cfe-dev mailing list
>> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160204/221fe237/attachment.html>


More information about the cfe-dev mailing list