On 05/03/11 23:31, John McCall wrote:
> A language option is overkill. Just make a new warning category and
> test whether that warning is enabled at the end of the translation unit.
> Unconditionally growing a set of constructors (or better yet, a set
> of classes) is not going to be prohibitive, especially as system headers
> are not expected to have any.
Sorry. My sleepy brain decided that "error"->"not a warning"->"not a
warning flag".
>> The PCH test is XFAILed because we currently can't deal with a note
>> emitted in the header
>> An expected-note on the corresponding line in the main file works.
> It's a hack, but it's one we've used elsewhere.
Ok. I'll also look at Sebastian's hack.
> Random other notes:
>> This const_cast is really ugly; why is it necessary in the first place?
Because we don't have a hasBody function returning a non-const Decl, and
I need to invalidate the nodes. Is there a better way? (I need the
definition as only the definition will have the CtorInitializer)
> If you're going use hasBody for its side-effect, at least cast the result to
> void or something. An assert that it worked would also be nice.
Ok.
> Use this:
> if (set.insert(thing)) { ...
> instead of this:
> if (!set.count(thing)) {
> set.insert(thing);
> ...
Ok.
>> SmallSet does not guarantee any particular iteration order; if it
> falls over to using a 'large' set and rehashes, your notes will be
> completely screwed up. Use llvm::SetVector instead, or just use a
> set (or the two-cursor algorithm) and then recompute the cycle
> once you've detected it.
The cycle is recomputed once detected.
> If you did this by class instead of by constructor, I think the
> bookkeeping in your algorithm would be much tidier.
The exact same bookkeeping would be required, as I might be processing
the constructors even in a given record, in any arbitrary order.
Sean