[cfe-dev] clang-tidy FixItHint::CreateRemoval -- trailing semicolon stays
Alexander Kornienko via cfe-dev
cfe-dev at lists.llvm.org
Thu Feb 4 07:39:14 PST 2016
On Thu, Feb 4, 2016 at 3:47 PM, Tim Halloran <tim.halloran at surelogic.com>
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.
>
You can supply many fixes to the same warning as long as they don't overlap.
>
> 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);
>
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 (
http://reviews.llvm.org/D16113) to use filename extensions for this purpose.
> }
>
> 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();
>
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.
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/e87cad0c/attachment.html>
More information about the cfe-dev
mailing list