[cfe-commits] r101756 - in /cfe/trunk: include/clang/AST/Decl.h lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenModule.h test/CodeGenCXX/static-local-in-local-class.cpp

[Daniel Dunbar]

On Mon, Apr 19, 2010 at 11:47 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>
> On Apr 19, 2010, at 10:21 AM, Daniel Dunbar wrote:
>
>> Hi Fariborz,
>>
>> On Sun, Apr 18, 2010 at 2:01 PM, Fariborz Jahanian <fjahanian at apple.com>
>> wrote:
>>>
>>> Author: fjahanian
>>> Date: Sun Apr 18 16:01:23 2010
>>> New Revision: 101756
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=101756&view=rev
>>> Log:
>>> Local static variables must be available module-wise
>>> as they are accessible in static methods in a class
>>> local to the same function. Fixes PR6769.
>>
>> Whee, I didn't even know that was legal. This patch has a couple
>> problems, the biggest one is that we are inserting *very* local decl
>> into an extra map now. This is bad from a performance perspective.
>>
>>
>> I don't see a very elegant solution to this yet, but perhaps it makes
>> sense for Sema to mark Decl's where the static function-local storage
>> must be promoted to the translation unit. Among other things this
>
> This moves the burden to the front-end shared by every one. I prefer to
> have the clients do the extra lifting if they need to.

I don't understand what you mean, moving this to the front-end means
that every decl can have a very simple predicate which is used by all
clients. This *reduces* the burden on clients, not increases it.

>>
>> means we wouldn't be sticking these things in the extra map for the
>> non-C++ path.
>
> I did the insertion c++ specific for performance.

I'm still not happy with this. It still penalizes all local decls,
when this is a case that almost never happens. It is much better to
have Sema clearly define the properties of things, and just let IRgen
handle implementing them.

 - Daniel

>>
>>
>> Some other nits below...
>
> In r101794.
>
> - Thanks, Fariborz
>
>