On May 3, 2011, at 10:57 PM, Sean Hunt wrote:
> Author: coppro
> Date: Wed May 4 00:57:24 2011
> New Revision: 130836
>
> URL: http://llvm.org/viewvc/llvm-project?rev=130836&view=rev
> Log:
> Implement a better version of delegating constructor cycle detection.
>
> This is more efficient as it's all done at once at the end of the TU.
> This could still get expensive, so a flag is provided to disable it. As
> an added bonus, the diagnostics will now print out a cycle.
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.
> 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.
Random other notes:
This const_cast is really ugly; why is it necessary in the first place?
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.
Use this:
if (set.insert(thing)) { ...
instead of this:
if (!set.count(thing)) {
set.insert(thing);
...
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.
If you did this by class instead of by constructor, I think the
bookkeeping in your algorithm would be much tidier.
John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
