[cfe-commits] r170616 - in /cfe/trunk: lib/Lex/TokenLexer.cpp test/Preprocessor/macro_arg_slocentry_merge.c test/Preprocessor/macro_arg_slocentry_merge.h

[Argyrios Kyrtzidis]

On Dec 20, 2012, at 11:09 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Wed, Dec 19, 2012 at 3:55 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> Author: akirtzidis
>> Date: Wed Dec 19 17:55:44 2012
>> New Revision: 170616
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=170616&view=rev
>> Log:
>> [preprocessor] When "merging" macro argument tokens into one SLocEntry chunk,
>> make sure they came from the same kind of FileIDs.
>> 
>> Thanks to Abramo Bagnara for providing the test case.
>> 
>> Added:
>>    cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c
>>    cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.h
>> Modified:
>>    cfe/trunk/lib/Lex/TokenLexer.cpp
>> 
>> Modified: cfe/trunk/lib/Lex/TokenLexer.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/TokenLexer.cpp?rev=170616&r1=170615&r2=170616&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Lex/TokenLexer.cpp (original)
>> +++ cfe/trunk/lib/Lex/TokenLexer.cpp Wed Dec 19 17:55:44 2012
>> @@ -749,14 +749,18 @@
>> 
>>   Token *NextTok = begin_tokens + 1;
>>   for (; NextTok < end_tokens; ++NextTok) {
>> +    SourceLocation NextLoc = NextTok->getLocation();
>> +    if (CurLoc.isFileID() != NextLoc.isFileID())
>> +      break; // Token from different kind of FileID.
> 
> Is it actually worthwhile to perform this merging for tokens with
> differing FileIDs (or for tokens where the offset in the spelling
> SLocEntry isn't the same as the offset in the expansion SLocEntry)?
> Allowing the spelling for a macro arg expansion to walk between
> SLocEntries seems to *significantly* complicate the semantic model
> here, and I would be surprised if actually happens frequently enough
> to be a valuable saving.

I fully appreciate (and share) your concern, but it provides significant savings in memory and PCH size when using some parts of boost.
And resolving locations to FileIDs in that part of the code affects performance.

> 
>> +
>>     int RelOffs;
>> -    if (!SM.isInSameSLocAddrSpace(CurLoc, NextTok->getLocation(), &RelOffs))
>> +    if (!SM.isInSameSLocAddrSpace(CurLoc, NextLoc, &RelOffs))
>>       break; // Token from different local/loaded location.
>>     // Check that token is not before the previous token or more than 50
>>     // "characters" away.
>>     if (RelOffs < 0 || RelOffs > 50)
>>       break;
>> -    CurLoc = NextTok->getLocation();
>> +    CurLoc = NextLoc;
>>   }
>> 
>>   // For the consecutive tokens, find the length of the SLocEntry to contain
>> 
>> Added: cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c?rev=170616&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c (added)
>> +++ cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c Wed Dec 19 17:55:44 2012
>> @@ -0,0 +1,7 @@
>> +// RUN: not %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck %s
>> +
>> +#include "macro_arg_slocentry_merge.h"
>> +
>> +// CHECK: macro_arg_slocentry_merge.h:7:19: error: unknown type name 'win'
>> +// CHECK: macro_arg_slocentry_merge.h:5:16: note: expanded from macro 'WINDOW'
>> +// CHECK: macro_arg_slocentry_merge.h:6:18: note: expanded from macro 'P_'
>> 
>> Added: cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.h?rev=170616&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.h (added)
>> +++ cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.h Wed Dec 19 17:55:44 2012
>> @@ -0,0 +1,7 @@
>> +
>> +
>> +
>> +
>> +#define WINDOW win
>> +#define P_(args) args
>> +extern void f P_((WINDOW win));
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits