[cfe-dev] clang-tidy FixItHint::CreateRemoval -- trailing semicolon stays
Kirill Bobyrev via cfe-dev
cfe-dev at lists.llvm.org
Thu Feb 4 07:33:36 PST 2016
Hi Tim,
Iterating through the SourceLocations is a nice solution, but I think the check might accidentally catch the semicolon in the comment IIUC.
Example:
using namespace std /* ; */ ;
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”.
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.
—
Kirill Bobyrev
> On 04 Feb 2016, at 5:47 PM, Tim Halloran via cfe-dev <cfe-dev at lists.llvm.org> 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 <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 <mailto:cfe-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <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/ffd38dbc/attachment.html>
More information about the cfe-dev
mailing list