[cfe-commits] r130836 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/LangOptions.h include/clang/Driver/CC1Options.td include/clang/Sema/Sema.h lib/Frontend/CompilerInvocation.cpp lib/Sema/Sema.cpp lib/Sema/SemaDeclCXX.cpp test/PCH/cxx0x-delegating-ctors.cpp test/PCH/cxx0x-delegating-ctors.h test/SemaCXX/cxx0x-delegating-ctors.cpp

[John McCall]

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.