Bug with FastFoldingSetNode / FoldingSetNodeID::Add

[RealLitb]

Bugzilla Link	9499
Resolution	FIXED
Resolved on	Apr 26, 2011 12:20
Version	trunk
OS	Linux
Attachments	Proposed fix
CC	@lattner

Extended Description

There's a problem with interaction of those two functions. FoldingSetNodeID::Add is supposed to add data to a folding set node ID. However, when we add folding set node that derives from FastFoldingSetNode, its profile function will overwrite anything that has been added previously to the folding set node ID:

// FoldingSetNodeID::Add
template
inline void Add(const T& x) { FoldingSetTrait::Profile(x, *this); }

// FastFoldingSetNode::Profile
void Profile(FoldingSetNodeID& ID) const { ID = FastID; }

I think that FoldingSetNodeID::Add should let the Profile function add stuff to a temporary folding set node id and then append all collected data to "*this".

[RealLitb]

The fix I propose takes a different approach. It leaves FoldingSetNodeID::Add alone (so the respective Profile function can choose the behavior itself).

Instead, it changes FastFoldingSetNode to use FoldingSetNodeID::AddNodeID (new function), which appends the bits of another NodeID to itself.

[lattner]

Applied in r130151, thanks. Please just send patches to llvm-commits instead of filing bugzillas for them.

[RealLitb]

Applied in r130151, thanks. Please just send patches to llvm-commits instead
of filing bugzillas for them.

Thanks, I will do next time :) I sent it to llvm-dev, but it received no replies.

There was another problematic one: http://comments.gmane.org/gmane.comp.compilers.llvm.devel/38916 . Should I resend that to llvm-commits so it can be discussed there? Or does the function behave as designed and the design makes sense?

[RealLitb]

adding Chris for my follow-up question.

[lattner]

Yep, please send patches to llvm-commits, thanks!