[cfe-commits] r146430 - in /cfe/trunk: lib/Sema/SemaChecking.cpp test/SemaCXX/Inputs/array-bounds-system-header.h test/SemaCXX/array-bounds-system-header.cpp

[Argyrios Kyrtzidis]

On Dec 12, 2011, at 2:57 PM, Matt Beaumont-Gay wrote:

> Argyrios, a couple of questions came up while I was working on this
> patch, and Chandler suggested you were the right person to ask:
> 
> On Mon, Dec 12, 2011 at 14:35, Matt Beaumont-Gay <matthewbg at google.com> wrote:
>> @@ -4381,6 +4381,16 @@
>>     switch (expr->getStmtClass()) {
>>       case Stmt::ArraySubscriptExprClass: {
>>         const ArraySubscriptExpr *ASE = cast<ArraySubscriptExpr>(expr);
>> +        // Suppress the warning if the subscript expression (as identified by
>> +        // the ']' location) and the index expression are both from macro
>> +        // expansions within a system header.
>> +        SourceLocation RBracketLoc = SourceMgr.getSpellingLoc(
>> +            ASE->getRBracketLoc());
>> +        SourceLocation IndexLoc = SourceMgr.getSpellingLoc(
>> +            ASE->getIdx()->IgnoreParens()->getLocStart());
>> +        if (SourceMgr.isFromSameFile(RBracketLoc, IndexLoc) &&
>> +            SourceMgr.isInSystemHeader(RBracketLoc))
> 
> It might be nice to tighten this check down to check whether the
> subscript expression and the index expression come from the same
> macro. Is there an API for this? I didn't see anything obvious in
> SourceManager.

After checking that both locations are in macros

if (RBracketLoc.isMacroID() && IndexLoc.isMacroID())

to check whether they came from the same top-level macro:

if (SourceMgr.getExpansionLoc(RBracketLoc) == SourceMgr.getExpansionLoc(IndexLoc))


if you want to check whether they came from the same most-nested macro, you would first have to do for both locations:

  while (SourceMgr.isMacroArgExpansion(Loc))
    Loc = SourceMgr.getImmediateExpansionRange(Loc).first;

then compare their

SourceMgr.getImmediateExpansionRange(Loc).first

Probably a good idea to add a method in SourceManager to do the above, like 'getImmediateMacroExpansionLoc' or something.


> 
>> Added: cfe/trunk/test/SemaCXX/Inputs/array-bounds-system-header.h
> 
> I tried to test this by faking up a system header with line directives
> in an existing test file, but it looks like isFromSameFile doesn't
> respect line directives. Is this a bug or a feature?

Only a couple of methods in SourceManager respect line directives, since it's expensive.
'isFromSameFile' should really be renamed to 'isFromSameFileID' to make it more clear what it does, and If you would want such a method that takes into account line directives, I'd suggest adding a new one in SourceManager, instead of modifying 'isFromSameFile'.

-Argyrios

> 
> Thanks,
> Matt
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits