[cfe-commits] [PATCH] Macro history (de-)serialization.

[Alexander Kornienko]

On Fri, Sep 14, 2012 at 7:46 AM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Sep 12, 2012, at 1:58 PM, Alexander Kornienko <
> reviews at llvm-reviews.chandlerc.com> wrote:
>
> +    // History of macro definitions for this identifier in chronological
> order.
> +    SmallVector<MacroInfo*, 8> MacroHistory;
> +    while (MI) {
> +      MacroHistory.push_back(MI);
> +      MI = MI->getPreviousDefinition();
> +    }
> +
> +    while (!MacroHistory.empty()) {
> +      MI = MacroHistory.pop_back_val();
>
> While this will write out all of the macro definitions and undefs, it has
> a few issues:
>
>   - There's no way for the reader to know that more than one macro
> definition is coming, and in fact the reader looks like it will only end up
> reading the *first* macro definition, rather than the *last* one. I suspect
> that #define'ing then #undef'ing a macro and defining it again within a PCH
> file will lead to the wrong macro definition in a client of that PCH file
>

This seems to work correctly, at least the test I've added, should have
broken if it didn't.


>   - It forces the reader to load in all of the macro history to get to the
> most recent one, which will be wasted effort in most cases: the vast
> majority of clients only care about the most recent macro definition and
> the rest of the definitions could be loaded lazily (e.g., the way we lazily
> load the body of a function).


You mean adding another level of laziness
above  ExternalPreprocessorSource::LoadMacroDefinition & Co? Are there
reasons to believe it will help to solve more problems than it creates?


>   - This always writes out all of the macro history, even in chained PCH,
> where some of the macro history might already be in the original PCH. (This
> kind of issue matters more for modules).
>

Any ideas how to fix this?

-- 
Regards,
Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120924/7088bf95/attachment.html>