[PATCH] PR11306 - Variadic template fix-it suggestion

Richard Smith richard at metafoo.co.uk
Wed Jun 4 18:02:37 PDT 2014


================
Comment at: lib/Parse/ParseTemplate.cpp:674
@@ +673,3 @@
+                                       Declarator *D) {
+  if (EllipsisLoc.isValid()) {
+    FixItHint Insertion;
----------------
Please move this check to the callers in `ParseDecl.cpp`, and change this to an assert.

================
Comment at: lib/Parse/ParseTemplate.cpp:676-679
@@ +675,6 @@
+    FixItHint Insertion;
+    if (D && D->getEllipsisLoc().isInvalid()) {
+      Insertion = FixItHint::CreateInsertion(CorrectLoc, "...");
+      D->setEllipsisLoc(EllipsisLoc);
+    }
+    else if (!D && CorrectLoc.isValid())
----------------
I don't really like the interface of this function:

 * It's very `Declarator`-specific, and yet is called in non-`Declarator` cases
 * It gives the wrong fixit in the template-type-parameter and template-template-parameter cases because it can't tell whether there was already an ellipsis.

Maybe split it into:

  DiagnoseMisplacedEllipsis(SourceLocation NewLoc, SourceLocation CorrectLoc, bool AlreadyHasEllipsis);
  DiagnoseMisplacedEllipsisInDeclarator(SourceLocation NewLoc, SourceLocation CorrectLoc, Declarator *D);

... with the second function calling the first?

================
Comment at: test/FixIt/fixit-cxx0x.cpp:142
@@ +141,3 @@
+
+namespace MissplacedParameterPack {
+  template <typename Args...> // expected-error {{'...' must immediately precede declared identifier}}
----------------
Typo `Missplaced`

http://reviews.llvm.org/D3604






More information about the cfe-commits mailing list