[cfe-dev] clang-tidy FixItHint::CreateRemoval -- trailing semicolon stays
Tim Halloran via cfe-dev
cfe-dev at lists.llvm.org
Fri Feb 5 09:36:50 PST 2016
Uggh, disregard the below -- it appears the clang-tidy infrastructure
handles nested files. Is there some sort of AST filtering in matching?
Basically, I copied the code from (trunk) from
google/UnnamedNamespaceInHeaderCheck and it seems to behave okay (even when
targeted at header files.
On Fri, Feb 5, 2016 at 11:42 AM, Tim Halloran <tim.halloran at surelogic.com>
wrote:
> 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
>
--
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/25b61fe7/attachment.html>
More information about the cfe-dev
mailing list