[cfe-dev] clang-tidy FixItHint::CreateRemoval -- trailing semicolon stays
Tim Halloran via cfe-dev
cfe-dev at lists.llvm.org
Fri Feb 5 08:42:31 PST 2016
On Thu, Feb 4, 2016 at 10:39 AM, Alexander Kornienko <alexfh at google.com>
wrote:
> On Thu, Feb 4, 2016 at 3:47 PM, Tim Halloran <tim.halloran at surelogic.com>
> wrote:
>
>>
>> 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.
>
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).
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?
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.
Thanks for the pointer!
Best,
Tim
>
>> }
>>
>> 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
>>
>
>
--
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/20160205/41f427cd/attachment.html>
More information about the cfe-dev
mailing list