[C++ Patch] More location fixes to grokdeclarator

[Paolo Carlini]

Hi,

On 28/06/2018 01:31, Jason Merrill wrote:
>
>> +/* Returns the smallest location.  */
> This should probably say "...that is not UNKNOWN_LOCATION."
I agree.
> Actually, the places you use min_location would seem to work fine with
> max_location as well.  What are your criteria for choosing one or the
> other?
I should have explained that in better detail. I see two different 
circumstances: either we have error messages where we say something like 
"cannot be both":

-	  error ("member %qD cannot be declared both %<virtual%> "
-		 "and %<static%>", dname);
+	  error_at (max_location (declspecs->locations[ds_virtual],
+				  declspecs->locations[ds_storage_class]),
+		    "member %qD cannot be declared both %<virtual%> "
+		    "and %<static%>", dname);

where, in my opinion, we want to point to the max_location, we want to point to where the contradiction shows up in the code. Or, we have errors like:

-	  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);

where ill-formed code has either one or two such specifiers (that is __thread and/or static) but even one would wrong, thus we want to point to the first one, thus min_location (this is in fact a variant of the reasoning behind smallest_type_quals_location).

Did I explain myself clearly enough? If we are going for something simple, I would suggest uniformly using min_location, not max_location.

By the way, I think I found examples of locations following both the patterns above in clang and icc too.

Thanks,
Paolo.