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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to