r244416 - [modules] PR22534: Load files specified by -fmodule-file= eagerly. In particular, this avoids the need to re-parse module map files when using such a module.

Gábor Horváth via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 18 15:06:46 PDT 2015


On 18 August 2015 at 14:46, Richard Smith <richard at metafoo.co.uk> wrote:

> On Tue, Aug 18, 2015 at 1:52 PM, Gábor Horváth <xazax.hun at gmail.com>
> wrote:
>
>> On 18 August 2015 at 13:41, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>>> On Tue, Aug 18, 2015 at 12:59 PM, Gábor Horváth <xazax.hun at gmail.com>
>>> wrote:
>>>
>>>> Hi!
>>>>
>>>> In r244416 you made createModuleManager to call the Initialize method
>>>> of the ASTConsumer.
>>>> Because of this change the ASTConsumer::Initialize might be called
>>>> twice in some scenarios.
>>>>
>>>> This change makes the Static Analyzer crash (use after free) in those
>>>> scenarios. I think most of the ASTConsumers was not written with that in
>>>> mind Initialize might be called twice for the object. This fact is also not
>>>> covered by the documentation.
>>>>
>>>> In my opinion we should either guarantee that the Initialize method
>>>> will be called only once during the lifetime of an ASTConsumer, or document
>>>> the fact that, it might be called multiple times and notify the authors of
>>>> the different consumers to update their classes accordingly (announcement
>>>> on the mailing list?).
>>>>
>>>> What do you think?
>>>>
>>>
>>> Fixed in r245346.
>>>
>>> It seems like an indicator of poor design that we have the initialize
>>> function at all. I don't think there is any situation where we need or want
>>> to create an ASTConsumer before we have enough information to create the
>>> ASTContext, so perhaps we can just get rid of this function.
>>>
>>
>> Thank you for fixing this!
>>
>> I agree that it would be awesome to get rid of that function.
>>
>
> CCing the right list again :)
>
> Are you interested in looking into this? (If not, it's going to near the
> bottom of a long list of TODO items for me...)
>

Hope to CC the right list this time :)

Well, it would end up on the bottom of my TODO list as well, so maybe we
should both add it, and the one who gets there earlier should do this.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150818/8600bee0/attachment-0001.html>


More information about the cfe-commits mailing list