Hi,

On 28/06/2018 03:22, David Malcolm wrote:
[snip]
If I'm following you right, the idea is that gcc should complain
because two different things in the user's source code contradict
each
other.

In such circumstances, I think we ought to try to print *both*
locations, so that we're showing, rather than just telling.
Or to put in another way, if two things in the user's source contradict
each other, we should show the user both.  The user is going to have to
decide to delete one (or both) of them, and we don't know which one,
but at least by showing both it helps him/her take their next action.
Sure, makes sense. Thus the below uses rich_location the way you explained. I also added 2 specific testcases and extended a bit another one to exercise a bit more min_location..Of course the patch doesn't add max_location anymore, I suspect we are not going to find uses for a max anytime soon, because we really want rich_location with multiple ranges in all those cases...

Thanks!
Paolo.

/////////////////////////


Index: cp/decl.c
===================================================================
--- cp/decl.c   (revision 262214)
+++ cp/decl.c   (working copy)
@@ -8545,15 +8545,18 @@ check_concept_fn (tree fn)
 {
   // A constraint is nullary.
   if (DECL_ARGUMENTS (fn))
-    error ("concept %q#D declared with function parameters", fn);
+    error_at (DECL_SOURCE_LOCATION (fn),
+             "concept %q#D declared with function parameters", fn);
 
   // The declared return type of the concept shall be bool, and
   // it shall not be deduced from it definition.
   tree type = TREE_TYPE (TREE_TYPE (fn));
   if (is_auto (type))
-    error ("concept %q#D declared with a deduced return type", fn);
+    error_at (DECL_SOURCE_LOCATION (fn),
+             "concept %q#D declared with a deduced return type", fn);
   else if (type != boolean_type_node)
-    error ("concept %q#D with non-%<bool%> return type %qT", fn, type);
+    error_at (DECL_SOURCE_LOCATION (fn),
+             "concept %q#D with non-%<bool%> return type %qT", fn, type);
 }
 
 /* Helper function.  Replace the temporary this parameter injected
@@ -9808,16 +9811,32 @@ smallest_type_quals_location (int type_quals, cons
     loc = locations[ds_const];
 
   if ((type_quals & TYPE_QUAL_VOLATILE)
-      && (loc == UNKNOWN_LOCATION || locations[ds_volatile] < loc))
+      && (loc == UNKNOWN_LOCATION
+         || linemap_location_before_p (line_table,
+                                       locations[ds_volatile], loc)))
     loc = locations[ds_volatile];
 
   if ((type_quals & TYPE_QUAL_RESTRICT)
-      && (loc == UNKNOWN_LOCATION || locations[ds_restrict] < loc))
+      && (loc == UNKNOWN_LOCATION
+         || linemap_location_before_p (line_table,
+                                       locations[ds_restrict], loc)))
     loc = locations[ds_restrict];
 
   return loc;
 }
 
+/* Returns the smallest location that is not UNKNOWN_LOCATION.  */
+
+static location_t
+min_location (location_t loca, location_t locb)
+{
+  if (loca == UNKNOWN_LOCATION
+      || (locb != UNKNOWN_LOCATION
+         && linemap_location_before_p (line_table, locb, loca)))
+    return locb;
+  return loca;
+}
+
 /* Check that it's OK to declare a function with the indicated TYPE
    and TYPE_QUALS.  SFK indicates the kind of special function (if any)
    that this function is.  OPTYPE is the type given in a conversion
@@ -10710,14 +10729,20 @@ grokdeclarator (const cp_declarator *declarator,
     {
       if (staticp == 2)
        {
-         error ("member %qD cannot be declared both %<virtual%> "
-                "and %<static%>", dname);
+         rich_location richloc (line_table, declspecs->locations[ds_virtual]);
+         richloc.add_range (declspecs->locations[ds_storage_class], false);
+         error_at (&richloc, "member %qD cannot be declared both %<virtual%> "
+                   "and %<static%>", dname);
          storage_class = sc_none;
          staticp = 0;
        }
       if (constexpr_p)
-       error ("member %qD cannot be declared both %<virtual%> "
-              "and %<constexpr%>", dname);
+       {
+         rich_location richloc (line_table, declspecs->locations[ds_virtual]);
+         richloc.add_range (declspecs->locations[ds_constexpr], false);
+         error_at (&richloc, "member %qD cannot be declared both %<virtual%> "
+                   "and %<constexpr%>", dname);
+       }
     }
   friendp = decl_spec_seq_has_spec_p (declspecs, ds_friend);
 
@@ -10726,18 +10751,27 @@ grokdeclarator (const cp_declarator *declarator,
     {
       if (typedef_p)
        {
-         error ("typedef declaration invalid in parameter declaration");
+         error_at (declspecs->locations[ds_typedef],
+                   "typedef declaration invalid in parameter declaration");
          return error_mark_node;
        }
       else if (template_parm_flag && storage_class != sc_none)
        {
-         error ("storage class specified for template parameter %qs", name);
+         error_at (min_location (declspecs->locations[ds_thread],
+                                 declspecs->locations[ds_storage_class]),
+                   "storage class specified for template parameter %qs",
+                   name);
          return error_mark_node;
        }
       else if (storage_class == sc_static
               || storage_class == sc_extern
               || thread_p)
-       error ("storage class specifiers invalid in parameter declarations");
+       {
+         error_at (min_location (declspecs->locations[ds_thread],
+                                 declspecs->locations[ds_storage_class]),
+                   "storage class specified for parameter %qs", name);
+         return error_mark_node;
+       }
 
       /* Function parameters cannot be concept. */
       if (concept_p)
@@ -10872,13 +10906,19 @@ grokdeclarator (const cp_declarator *declarator,
       else
        {
          if (decl_context == FIELD)
-           error ("storage class specified for %qs", name);
+           error_at (min_location (declspecs->locations[ds_thread],
+                                   declspecs->locations[ds_storage_class]),
+                     "storage class specified for %qs", name);
          else
            {
              if (decl_context == PARM || decl_context == CATCHPARM)
-               error ("storage class specified for parameter %qs", name);
+               error_at (min_location (declspecs->locations[ds_thread],
+                                       declspecs->locations[ds_storage_class]),
+                         "storage class specified for parameter %qs", name);
              else
-               error ("storage class specified for typename");
+               error_at (min_location (declspecs->locations[ds_thread],
+                                       declspecs->locations[ds_storage_class]),
+                         "storage class specified for typename");
            }
          if (storage_class == sc_register
              || storage_class == sc_auto
@@ -10900,7 +10940,8 @@ grokdeclarator (const cp_declarator *declarator,
           && storage_class != sc_static)
     {
       if (declspecs->gnu_thread_keyword_p)
-       pedwarn (input_location, 0, "function-scope %qs implicitly auto and "
+       pedwarn (declspecs->locations[ds_thread],
+                0, "function-scope %qs implicitly auto and "
                 "declared %<__thread%>", name);
 
       /* When thread_local is applied to a variable of block scope the
@@ -10912,7 +10953,10 @@ grokdeclarator (const cp_declarator *declarator,
 
   if (storage_class && friendp)
     {
-      error ("storage class specifiers invalid in friend function 
declarations");
+      error_at (min_location (declspecs->locations[ds_thread],
+                             declspecs->locations[ds_storage_class]),
+               "storage class specifiers invalid in friend function "
+               "declarations");
       storage_class = sc_none;
       staticp = 0;
     }
@@ -11238,7 +11282,8 @@ grokdeclarator (const cp_declarator *declarator,
                if (virtualp)
                  {
                    /* Cannot be both friend and virtual.  */
-                   error ("virtual functions cannot be friends");
+                   error_at (declspecs->locations[ds_friend],
+                             "virtual functions cannot be friends");
                    friendp = 0;
                  }
                if (decl_context == NORMAL)
@@ -12369,15 +12414,18 @@ grokdeclarator (const cp_declarator *declarator,
        else if (thread_p)
          {
            if (declspecs->gnu_thread_keyword_p)
-             error ("storage class %<__thread%> invalid for function %qs",
-                    name);
+             error_at (declspecs->locations[ds_thread],
+                       "storage class %<__thread%> invalid for function %qs",
+                       name);
            else
-             error ("storage class %<thread_local%> invalid for function %qs",
-                    name);
+             error_at (declspecs->locations[ds_thread],
+                       "storage class %<thread_local%> invalid for "
+                       "function %qs", name);
          }
 
         if (virt_specifiers)
-          error ("virt-specifiers in %qs not allowed outside a class 
definition", name);
+          error ("virt-specifiers in %qs not allowed outside a class "
+                "definition", name);
        /* Function declaration not at top level.
           Storage classes other than `extern' are not allowed
           and `extern' makes no difference.  */
Index: testsuite/g++.dg/concepts/fn-concept2.C
===================================================================
--- testsuite/g++.dg/concepts/fn-concept2.C     (revision 262214)
+++ testsuite/g++.dg/concepts/fn-concept2.C     (working copy)
@@ -1,7 +1,10 @@
 // { dg-options "-std=c++17 -fconcepts" }
 
 template<typename T>
-  concept auto C1() { return 0; } // { dg-error "deduced return type" }
+  concept auto C1() { return 0; } // { dg-error "16:concept .concept auto 
C1\\(\\). declared with a deduced return type" }
 
 template<typename T>
-  concept int C2() { return 0; } // { dg-error "return type" }
+  concept int C2() { return 0; } // { dg-error "15:concept .concept int 
C2\\(\\). with non-.bool. return type .int." }
+
+template<typename T>
+  concept bool C3(int) { return 0; } // { dg-error "16:concept .concept bool 
C3\\(int\\). declared with function parameters" }
Index: testsuite/g++.dg/cpp0x/constexpr-virtual5.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-virtual5.C (revision 262214)
+++ testsuite/g++.dg/cpp0x/constexpr-virtual5.C (working copy)
@@ -2,5 +2,5 @@
 // { dg-do compile { target c++11 } }
 
 struct S {
-  constexpr virtual int f() { return 1; }  // { dg-error "both 'virtual' and 
'constexpr'" }
+  constexpr virtual int f() { return 1; }  // { dg-error "13:member .f. cannot 
be declared both .virtual. and .constexpr." }
 };
Index: testsuite/g++.dg/cpp0x/pr51463.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr51463.C    (revision 262214)
+++ testsuite/g++.dg/cpp0x/pr51463.C    (working copy)
@@ -3,5 +3,6 @@
 
 struct A
 {
-  static virtual int i = 0;    // { dg-error "both 'virtual' and 
'static'|declared as" }
+  static virtual int i = 0;    // { dg-error "10:member .i. cannot be declared 
both .virtual. and .static." }
+  // { dg-error "declared as" "" { target *-*-* } .-1 }
 };
Index: testsuite/g++.dg/diagnostic/virtual-constexpr.C
===================================================================
--- testsuite/g++.dg/diagnostic/virtual-constexpr.C     (nonexistent)
+++ testsuite/g++.dg/diagnostic/virtual-constexpr.C     (working copy)
@@ -0,0 +1,16 @@
+// { dg-options "-fdiagnostics-show-caret" }
+// { dg-do compile { target c++11 } }
+
+struct S
+{
+  virtual constexpr void foo();  // { dg-error "3:member .foo. cannot be 
declared both .virtual. and .constexpr." }
+/* { dg-begin-multiline-output "" }
+   virtual constexpr void foo();
+   ^~~~~~~ ~~~~~~~~~
+   { dg-end-multiline-output "" } */
+  constexpr virtual void bar();  // { dg-error "13:member .bar. cannot be 
declared both .virtual. and .constexpr." }
+/* { dg-begin-multiline-output "" }
+   constexpr virtual void bar();
+   ~~~~~~~~~ ^~~~~~~
+   { dg-end-multiline-output "" } */
+};
Index: testsuite/g++.dg/diagnostic/virtual-static.C
===================================================================
--- testsuite/g++.dg/diagnostic/virtual-static.C        (nonexistent)
+++ testsuite/g++.dg/diagnostic/virtual-static.C        (working copy)
@@ -0,0 +1,15 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+struct S
+{
+  virtual static void foo();  // { dg-error "3:member .foo. cannot be declared 
both .virtual. and .static." }
+/* { dg-begin-multiline-output "" }
+   virtual static void foo();
+   ^~~~~~~ ~~~~~~
+   { dg-end-multiline-output "" } */
+  static virtual void bar();  // { dg-error "10:member .bar. cannot be 
declared both .virtual. and .static." }
+/* { dg-begin-multiline-output "" }
+   static virtual void bar();
+   ~~~~~~ ^~~~~~~
+   { dg-end-multiline-output "" } */
+};
Index: testsuite/g++.dg/other/locations1.C
===================================================================
--- testsuite/g++.dg/other/locations1.C (nonexistent)
+++ testsuite/g++.dg/other/locations1.C (working copy)
@@ -0,0 +1 @@
+void foo(static int p);  // { dg-error "10:storage class specified" }
Index: testsuite/g++.dg/other/typedef1.C
===================================================================
--- testsuite/g++.dg/other/typedef1.C   (revision 262214)
+++ testsuite/g++.dg/other/typedef1.C   (working copy)
@@ -1,7 +1,10 @@
 // PR c++/27572
 // { dg-do compile }
 
-void f1(typedef) {}        // { dg-error "no type|typedef declaration" }
-void f2(typedef x) {}      // { dg-error "type|typedef declaration" }
-void f3(typedef x[]) {}    // { dg-error "type|typedef declaration" }
-void f4(typedef int x) {}  // { dg-error "typedef declaration" }
+void f1(typedef) {}        // { dg-error "9:typedef declaration" }
+// { dg-error "no type" "" { target *-*-* } .-1 }
+void f2(typedef x) {}      // { dg-error "9:typedef declaration" }
+// { dg-error "type" "" { target *-*-* } .-1 }
+void f3(typedef x[]) {}    // { dg-error "9:typedef declaration" }
+// { dg-error "type" "" { target *-*-* } .-1 }
+void f4(typedef int x) {}  // { dg-error "9:typedef declaration" }
Index: testsuite/g++.dg/parse/dtor13.C
===================================================================
--- testsuite/g++.dg/parse/dtor13.C     (revision 262214)
+++ testsuite/g++.dg/parse/dtor13.C     (working copy)
@@ -3,6 +3,7 @@
 
 struct A
 {
-  static friend A::~A(); /* { dg-error "storage class specifiers|extra 
qualification|implicitly friend" } */
+  static friend A::~A(); /* { dg-error "3:storage class specifiers" } */
+  /* { dg-error "extra qualification|implicitly friend" "" { target *-*-* } 
.-1 } */
 };
 
Index: testsuite/g++.dg/template/error44.C
===================================================================
--- testsuite/g++.dg/template/error44.C (revision 262214)
+++ testsuite/g++.dg/template/error44.C (working copy)
@@ -1,7 +1,8 @@
 // PR c++/32056
 
-template <auto int T> struct A {}; // { dg-error "storage class specified|two 
or more" }
-template <extern int T> struct B {}; // { dg-error "storage class specified" }
-template <static int T> struct C {}; // { dg-error "storage class specified" }
-template <register int T> struct D {}; // { dg-error "storage class specified" 
}
-template <mutable int T> struct E {}; // { dg-error "storage class specified" }
+template <auto int T> struct A {}; // { dg-error "11:storage class specified" 
"" { target c++98_only } }
+// { dg-error "two or more" "" { target c++11 } .-1 }
+template <extern int T> struct B {}; // { dg-error "11:storage class 
specified" }
+template <static int T> struct C {}; // { dg-error "11:storage class 
specified" }
+template <register int T> struct D {}; // { dg-error "11:storage class 
specified" }
+template <mutable int T> struct E {}; // { dg-error "11:storage class 
specified" }
Index: testsuite/g++.dg/template/typedef4.C
===================================================================
--- testsuite/g++.dg/template/typedef4.C        (revision 262214)
+++ testsuite/g++.dg/template/typedef4.C        (working copy)
@@ -1,7 +1,8 @@
 // PR c++/27572
 // { dg-do compile }
 
-template<typedef> void foo();  // { dg-error "no type|typedef 
declaration|template" }
+template<typedef> void foo();  // { dg-error "10:typedef declaration" }
+// { dg-error "no type|template" "" { target *-*-* } .-1 }
 
 void bar()
 {
Index: testsuite/g++.dg/template/typedef5.C
===================================================================
--- testsuite/g++.dg/template/typedef5.C        (revision 262214)
+++ testsuite/g++.dg/template/typedef5.C        (working copy)
@@ -1,7 +1,10 @@
 // PR c++/27572
 // { dg-do compile }
 
-template<typedef,int>        struct A1; // { dg-error "no type|typedef 
declaration|default argument" }
-template<typedef x,int>      struct A2; // { dg-error "type|typedef 
declaration|default argument" }
-template<typedef x[],int>    struct A3; // { dg-error "no type|typedef 
declaration|expected" }
-template<typedef int x, int> struct A4; // { dg-error "typedef 
declaration|default argument" }
+template<typedef,int>        struct A1; // { dg-error "10:typedef declaration" 
}
+// { dg-error "no type|default argument" "" { target *-*-* } .-1 }
+template<typedef x,int>      struct A2; // { dg-error "10:typedef declaration" 
}
+// { dg-error "type|default argument" "" { target *-*-* } .-1 }
+template<typedef x[],int>    struct A3; // { dg-error "typedef declaration|no 
type|expected" }
+template<typedef int x, int> struct A4; // { dg-error "10:typedef declaration" 
}
+// { dg-error "default argument" "" { target *-*-* } .-1 }
Index: testsuite/g++.dg/tls/diag-2.C
===================================================================
--- testsuite/g++.dg/tls/diag-2.C       (revision 262214)
+++ testsuite/g++.dg/tls/diag-2.C       (working copy)
@@ -8,19 +8,19 @@ typedef __thread int g4;      /* { dg-error "multiple s
 
 void foo()
 {
-  __thread int l1;             /* { dg-error "implicitly auto and declared 
'__thread'" } */
+  __thread int l1;             /* { dg-error "3:function-scope .l1. implicitly 
auto and declared '__thread'" } */
   auto __thread int l2;                /* { dg-error "multiple storage 
classes|data types" } */
   __thread extern int l3;      /* { dg-error "'__thread' before 'extern'" } */
   register __thread int l4;    /* { dg-error "multiple storage classes" } */
 }                              /* { dg-error "ISO C\\+\\+17 does not allow 
'register' storage class specifier" "" { target c++17 } .-1 } */
 
-__thread void f1 ();           /* { dg-error "invalid for function" } */
-extern __thread void f2 ();    /* { dg-error "invalid for function" } */
-static __thread void f3 ();    /* { dg-error "invalid for function" } */
-__thread void f4 () { }                /* { dg-error "invalid for function" } 
*/
+__thread void f1 ();           /* { dg-error "1:storage class .__thread. 
invalid for function" } */
+extern __thread void f2 ();    /* { dg-error "8:storage class .__thread. 
invalid for function" } */
+static __thread void f3 ();    /* { dg-error "8:storage class .__thread. 
invalid for function" } */
+__thread void f4 () { }                /* { dg-error "1:storage class 
.__thread. invalid for function" } */
 
-void bar(__thread int p1);     /* { dg-error "(invalid in 
parameter)|(specified for parameter)" } */
+void bar(__thread int p1);     /* { dg-error "10:storage class specified for 
parameter" } */
 
 struct A {
-  __thread int i;              /* { dg-error "storage class specified" } */
+  __thread int i;              /* { dg-error "3:storage class specified" } */
 };
Index: testsuite/g++.dg/tls/locations1.C
===================================================================
--- testsuite/g++.dg/tls/locations1.C   (nonexistent)
+++ testsuite/g++.dg/tls/locations1.C   (working copy)
@@ -0,0 +1,4 @@
+/* { dg-require-effective-target tls } */
+
+template <__thread int T> struct F {}; // { dg-error "11:storage class 
specified" }
+template <static __thread int T> struct G {}; // { dg-error "11:storage class 
specified" }
Index: testsuite/g++.old-deja/g++.brendan/crash11.C
===================================================================
--- testsuite/g++.old-deja/g++.brendan/crash11.C        (revision 262214)
+++ testsuite/g++.old-deja/g++.brendan/crash11.C        (working copy)
@@ -9,13 +9,14 @@ class A {
        int     h;
        A() { i=10; j=20; }
        virtual void f1() { printf("i=%d j=%d\n",i,j); }
-       friend virtual void f2() { printf("i=%d j=%d\n",i,j); }// { dg-error "" 
}  virtual.*
+       friend virtual void f2() { printf("i=%d j=%d\n",i,j); } // { dg-error 
"2:virtual functions cannot be friends" }
 };
 
 class B : public A {
     public:
        virtual void f1() { printf("i=%d j=%d\n",i,j); }// { dg-error "" }  
member.*// ERROR -  member.*
-       friend virtual void f2() { printf("i=%d j=%d\n",i,j); }// { dg-error "" 
}  virtual.*// ERROR -  member.*// ERROR -  member.*
+       friend virtual void f2() { printf("i=%d j=%d\n",i,j); }  // { dg-error 
"2:virtual functions cannot be friends" }
+// { dg-error "private" "" { target *-*-* } .-1 }
 };
 
 int

Reply via email to