This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++ Patch] Three additional bitfield diagnostic tweaks (a regression fix included)


Hi,

On 06/12/18 16:11, Jason Merrill wrote:
2- Unfortunately I have to fix another buglet I recently introduced, completely similar to c++/88222 fixed by Marek. Well, at least we will not print anymore an empty '' when the unqualified_id is null because the field is unnamed.

-        error_at (declarator->id_loc,
-              "%qE is neither function nor member function; "
-              "cannot be declared friend", unqualified_id);
+        if (unqualified_id && declarator)
+          error_at (declarator->id_loc,
+                "%qE is neither function nor member function; "
+                "cannot be declared friend", unqualified_id);
+        else
+          error ("unnamed field is neither function nor member "
+             "function; cannot be declared friend");

I wonder if we want to use the 'name' variable here.

Well, the name variable doesn't seem that useful here because for the new testcase it has that famous catch all value "type name" .

I have been thinking that here and in other places we could imagine keeping only the declarator check and dropping the "name" check. Probably it would work. But in *many* existing places we actually check *only* the name thus I'm nervous about attempting that now...


3- In the non-static case too, when from grokdeclarator we are calling FIELD_DECL and passing the location as first argument, I think we want to likewise pass declarator->id_loc when available.

-        decl = build_decl (input_location,
+        decl = build_decl (declarator
+                   ? declarator->id_loc
+                   : input_location,

I think we want to put this in a local variable, to share with the static case and probably other places in grokdeclarator.

In the below I'm sharing it only with the static case, straightforward. Moving it one level up doesn't seem that useful because we only have rather safe IMHO unconditional uses either of input_location or of declarator->id_loc at the moment... Again, I'm pretty sure there is room for further clean-ups in this area, but, for 9, I'd rather take care of a bunch of additional small issues which I already have in my TODO list, in grokbitfield, for example, as already mentioned. By the way, if isn't already clear, I have been changing location bits only when I already have a set of testcases, constructed from our testsuite via (lenghty ;) instrumented runs.

New version of the patch attached.

Paolo.

Attachment: patch_grokbitfield2_2
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]