On Sep 24, 2012, at 6:41 AM, Alexander Kornienko <[email protected]> wrote:
> > > On Fri, Sep 14, 2012 at 7:46 AM, Douglas Gregor <[email protected]> wrote: > > On Sep 12, 2012, at 1:58 PM, Alexander Kornienko > <[email protected]> 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. Ah, now I see why. This MacroOffsets[Name] = Stream.GetCurrentBitNo(); keeps updating the offset for each successive macro definition, and since we're writing out the macro definitions in source order, the most recent bit offset gets placed into that table. Thanks for adding the test, though! > - 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? With precompiled headers, more laziness means better performance and a lower memory footprint. It's definitely worth doing well. The solution is most likely to turn MacroInfo's PreviousDefinition into a union of a MacroInfo* (if it's been loaded) and either an ID or an offset. See LazyOffsetPtr, which we use in several other places to provide laziness. > - 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? Typically we track whether the data was loaded from an AST file (e.g., a LoadedFromAST bit on MacroInfo), and then have a way (typically in the AST writer) to determine the pre-existing IDs for data with LoadedFromAST==true. That lets us tie together to different PCH files. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
