[cfe-commits] r165001 - in /cfe/trunk: include/clang/Serialization/ASTReader.h lib/Serialization/ASTReader.cpp lib/Serialization/ASTReaderDecl.cpp test/Modules/Inputs/templates-left.h test/Modules/Inputs/templates-right.h test/Modules/templates.mm

[Axel Naumann]

Hi Doug,

On 10/03/2012 08:38 PM, Douglas Gregor wrote:
> 
> On Oct 2, 2012, at 5:18 AM, Axel Naumann <Axel.Naumann at cern.ch> wrote:
> 
>> Author: axel
>> Date: Tue Oct  2 07:18:46 2012
>> New Revision: 165001
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=165001&view=rev
>> Log:
>> Only those InterestingDecls that got added to the AST should be passed to the ASTConsumer.
> 
> This doesn't seem necessary; ASTConsumers shouldn't care about seeing
> redeclarations of the same things.
> 
> Based on your test case, the real problem you're facing is that a
> function that gets defined in two different modules ends up with two
> bodies, a break in the invariants of the AST that results in a
> CodeGen assertion.
> 
> I went ahead and reverted the code in this change, leaving the test
> case, and put in a more appropriate fix that avoids giving functions
> two bodies. See r165137.

Thanks a lot, so much simpler!

Cheers, Axel.

>> Modified:
>>    cfe/trunk/include/clang/Serialization/ASTReader.h
>>    cfe/trunk/lib/Serialization/ASTReader.cpp
>>    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>    cfe/trunk/test/Modules/Inputs/templates-left.h
>>    cfe/trunk/test/Modules/Inputs/templates-right.h
>>    cfe/trunk/test/Modules/templates.mm
>>
>> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=165001&r1=165000&r2=165001&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
>> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Oct  2 07:18:46 2012
>> @@ -687,6 +687,12 @@
>>   /// Objective-C protocols.
>>   std::deque<Decl *> InterestingDecls;
>>
>> +  /// \brief Redecls that have been added to the AST
>> +  ///
>> +  /// Redecls that are deserialized but not in RedeclsAddedToAST must
>> +  /// not be passed to the ASTConsumers, even if they are InterestignDecls.
>> +  llvm::SmallPtrSet<Decl *, 16> RedeclsAddedToAST;
>> +
> 
> This is likely to be a fairly large set in practice. If we must have this functionality (which I'm not convinced of), we should strive to make it cheaper to implement.
> 
> 	- Doug
> 
> 
>