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

[Richard Smith]

On Fri, Dec 21, 2012 at 8:57 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> 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.

That makes sense, thank you!

>>
>>> +
>>>     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
>