[C++] PR 44516

Hi,

On 05/16/2012 05:41 PM, Jason Merrill wrote:
> On 05/16/2012 06:54 AM, Paolo Carlini wrote:
>> isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By
>> default should be interpreted as input_location, no?
> That would make sense to me; I don't know if it works that way now, 
> though.
So, in the meanwhile we learned that it doesn't.

I also carried out a simple practical experiment where I hacked my on 
going work 53371 and changed its "error (" to an "error_at 
(UNKNOWN_LOCATION, " and this is the change: this:

53371.C: In function ‘int main()’:
53371.C:11:15: error: cannot catch exceptions by rvalue reference
    } catch (int&&) { }
                ^

(the caret is below the first &, good enough, I guess) becomes:

In function ‘int main()’:
cc1plus: error: cannot catch exceptions by rvalue reference

totally broken. Thus, it remains to decide whether we want to guard 
against this with something local to the build_x_* functions, like my 
LOC_OR_HERE trick, or with something like the patchlet p1 which I'm 
attaching below. Just let me know.
>
>> @@ -11968,7 +11968,8 @@ tsubst_qualified_id (tree qualified_id, tree 
>> args,
>>    if (dependent_scope_p (scope))
>>      {
>>        if (is_template)
>> -    expr = build_min_nt (TEMPLATE_ID_EXPR, expr, template_args);
>> +    expr = build_min_nt_loc (UNKNOWN_LOCATION, TEMPLATE_ID_EXPR,
>> +                 expr, template_args);
>
> Here we should be able to retain the location from the 
> TEMPLATE_ID_EXPR we took apart earlier.
Ok. Something like p2 below?

We are getting close... ;)

Thanks,
Paolo.

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

Index: pt.c
===================================================================
--- pt.c	(revision 187588)
+++ pt.c	(working copy)
@@ -11932,6 +11932,7 @@ tsubst_qualified_id (tree qualified_id, tree args,
   tree name;
   bool is_template;
   tree template_args;
+  location_t loc = UNKNOWN_LOCATION;
 
   gcc_assert (TREE_CODE (qualified_id) == SCOPE_REF);
 
@@ -11940,6 +11941,7 @@ tsubst_qualified_id (tree qualified_id, tree args,
   if (TREE_CODE (name) == TEMPLATE_ID_EXPR)
     {
       is_template = true;
+      loc = EXPR_LOCATION (name);
       template_args = TREE_OPERAND (name, 1);
       if (template_args)
 	template_args = tsubst_template_args (template_args, args,
@@ -11968,7 +11970,7 @@ tsubst_qualified_id (tree qualified_id, tree args,
   if (dependent_scope_p (scope))
     {
       if (is_template)
-	expr = build_min_nt (TEMPLATE_ID_EXPR, expr, template_args);
+	expr = build_min_nt_loc (loc, TEMPLATE_ID_EXPR, expr, template_args);
       return build_qualified_name (NULL_TREE, scope, expr,
 				   QUALIFIED_NAME_IS_TEMPLATE (qualified_id));
     }

On 05/16/2012 08:34 PM, Paolo Carlini wrote:
> Ok. Something like p2 below?

Yes.  Since the earlier patch without LOC_OR_HERE passed testing, let's 
apply that plus this p2.

Does Manuel's suggestion of aborting if we get UNKNOWN_LOCATION for a 
diagnostic pass the testsuite?

Jason

On 05/17/2012 05:32 AM, Jason Merrill wrote:
> On 05/16/2012 08:34 PM, Paolo Carlini wrote:
>> Ok. Something like p2 below?
>
> Yes.  Since the earlier patch without LOC_OR_HERE passed testing, 
> let's apply that plus this p2.
Ok, great.
>
> Does Manuel's suggestion of aborting if we get UNKNOWN_LOCATION for a 
> diagnostic pass the testsuite?
Right, yesterday forgot to try that, when I came back to this task was a 
bit tired.

Now, the exercise turned out to be funny, because if I add a gcc_assert 
(location != UNKNOWN_LOCATION); at the beginning of 
diagnostic_report_diagnostic the bootstrap fails very early when 
configuring libgcc because the C compiler is not usable at all. The 
problem is that the following warning reaches the diagnostic machinery:

(gdb) p *diagnostic
$2 = {message = {format_spec = 0x38138b0 "-Wformat-y2k ignored without 
-Wformat", args_ptr = 0x7fffffffdaf8, err_no = 2, locus = 0x4fa94a3f, 
x_data = 0x0}, location = 0, override_column = 0, x_data = 0x0, kind = 
DK_WARNING, option_index = 226}

now, besides the specific warning - which right now I can't say to fully 
understand - it looks like we may sometimes try to output stuff which 
doesn't have to do with a specific line of the user code, thus doesn't 
come with a location. Can we concisely characterize those messages and 
exclude them from the gcc_assert?

Thanks,
Paolo.

On 05/17/2012 09:48 AM, Paolo Carlini wrote:
> Can we concisely characterize those messages and exclude them from the 
> gcc_assert?
Well, if don't quickly figure out something better, I guess we can 
always add the assert at the beginning of warning_at, error_at, etc.

Paolo.

On Thu, May 17, 2012 at 2:52 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 05/17/2012 09:48 AM, Paolo Carlini wrote:
>>
>> Can we concisely characterize those messages and exclude them from the
>> gcc_assert?
>
> Well, if don't quickly figure out something better, I guess we can always
> add the assert at the beginning of warning_at, error_at, etc.

I am still puzzled by why we need to assert, as opposed to just ignore, unless
we have a plan to make a wholesale move -- but even there I am bit nervous.

-- Gaby