RE: [Patch, AVR]: Hack around PR34734

RE: [Patch, AVR]: Hack around PR34734

[Weddington, Eric]

Hi Johann,

I understand your reasoning, but I'm not particularly fond of this hack.

Surely there's a way to fix this correctly without relying on this hack...

Eric Weddington

> -----Original Message-----
> From: Georg-Johann Lay [mailto:avr@gjlay.de]
> Sent: Wednesday, June 15, 2011 3:26 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Weddington, Eric; Anatoly Sokolov; Denis Chertykov
> Subject: [Patch, AVR]: Hack around PR34734
> 
> PR34734 is an annoying, false C++ warning for code like
> 
> const int x __attribute__((progmem)) = 1;
> 
> progmem.c:1:30: warning: only initialized variables can be placed into
> program memory area [enabled by default]
> 
> The problem is that DECL_INITIAL is NULL at the specific point in
> space and time (avr_handle_progmem_attribute) even though tree.def
> promises otherwise.
> 
> The patch hacks around by explicitly querying for C++ front end.
> 
> Johann
> 
> --
> 
> 	PR target/34734
> 	* config/avr/avr.c (avr_handle_progmem_attribute): Hack around
> 	non-present DECL_INITIAL if front end is C++.

Re: [Patch, AVR]: Hack around PR34734

[Georg-Johann Lay]

Weddington, Eric schrieb:
> Hi Johann,
> 
> I understand your reasoning, but I'm not particularly fond of this hack.

Yes, ACK. It's a hack to get rid of the PR.

> Surely there's a way to fix this correctly without relying on this hack...

Surely, gcc is man-made ;-) Someone will have to dive into C++ FE/parser.

Johann

> 
> Eric Weddington
> 
>> -----Original Message-----
>> From: Georg-Johann Lay [mailto:avr@gjlay.de]
>> Sent: Wednesday, June 15, 2011 3:26 PM
>> To: gcc-patches@gcc.gnu.org
>> Cc: Weddington, Eric; Anatoly Sokolov; Denis Chertykov
>> Subject: [Patch, AVR]: Hack around PR34734
>>
>> PR34734 is an annoying, false C++ warning for code like
>>
>> const int x __attribute__((progmem)) = 1;
>>
>> progmem.c:1:30: warning: only initialized variables can be placed into
>> program memory area [enabled by default]
>>
>> The problem is that DECL_INITIAL is NULL at the specific point in
>> space and time (avr_handle_progmem_attribute) even though tree.def
>> promises otherwise.
>>
>> The patch hacks around by explicitly querying for C++ front end.
>>
>> Johann
>>
>> --
>>
>> 	PR target/34734
>> 	* config/avr/avr.c (avr_handle_progmem_attribute): Hack around
>> 	non-present DECL_INITIAL if front end is C++.

[Patch, AVR]: Hack around PR34734

[Georg-Johann Lay]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

PR34734 is an annoying, false C++ warning for code like

const int x __attribute__((progmem)) = 1;

progmem.c:1:30: warning: only initialized variables can be placed into
program memory area [enabled by default]

The problem is that DECL_INITIAL is NULL at the specific point in
space and time (avr_handle_progmem_attribute) even though tree.def
promises otherwise.

The patch hacks around by explicitly querying for C++ front end.

Johann

--

	PR target/34734
	* config/avr/avr.c (avr_handle_progmem_attribute): Hack around
	non-present DECL_INITIAL if front end is C++.

[-- Attachment #2: pr34734.diff --]
[-- Type: text/x-patch, Size: 1017 bytes --]

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(Revision 175036)
+++ config/avr/avr.c	(Arbeitskopie)
@@ -5099,7 +5099,15 @@ avr_handle_progmem_attribute (tree *node
 	}
       else if (TREE_STATIC (*node) || DECL_EXTERNAL (*node))
 	{
-	  if (DECL_INITIAL (*node) == NULL_TREE && !DECL_EXTERNAL (*node))
+	  if (DECL_INITIAL (*node) == NULL_TREE && !DECL_EXTERNAL (*node)
+              /* FIXME: Despite documentation in tree.def,
+                 DECL_INITIAL is NULL if an initializer is
+                 present in C++.  This is presumably due to
+                 different parsers for C resp. C++.
+                 We hack around that annoying warning (PR34734)
+                 by quering for the front end and emit a warning
+                 just for non-C++.  */
+              && NULL == strcasestr (lang_hooks.name, "c++"))
 	    {
 	      warning (0, "only initialized variables can be placed into "
 		       "program memory area");

Re: [Patch, AVR]: Hack around PR34734

[Richard Guenther]

On Wed, Jun 15, 2011 at 3:54 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
> Weddington, Eric schrieb:
>> Hi Johann,
>>
>> I understand your reasoning, but I'm not particularly fond of this hack.
>
> Yes, ACK. It's a hack to get rid of the PR.
>
>> Surely there's a way to fix this correctly without relying on this hack...
>
> Surely, gcc is man-made ;-) Someone will have to dive into C++ FE/parser.

Well, doing this error handling at attribute parsing time is going to
be fragile.
I suggest to move it to code emission time instead (there currently isn't a
target hook for when the frontend finished a variable declaration).

Richard.

> Johann
>
>>
>> Eric Weddington
>>
>>> -----Original Message-----
>>> From: Georg-Johann Lay [mailto:avr@gjlay.de]
>>> Sent: Wednesday, June 15, 2011 3:26 PM
>>> To: gcc-patches@gcc.gnu.org
>>> Cc: Weddington, Eric; Anatoly Sokolov; Denis Chertykov
>>> Subject: [Patch, AVR]: Hack around PR34734
>>>
>>> PR34734 is an annoying, false C++ warning for code like
>>>
>>> const int x __attribute__((progmem)) = 1;
>>>
>>> progmem.c:1:30: warning: only initialized variables can be placed into
>>> program memory area [enabled by default]
>>>
>>> The problem is that DECL_INITIAL is NULL at the specific point in
>>> space and time (avr_handle_progmem_attribute) even though tree.def
>>> promises otherwise.
>>>
>>> The patch hacks around by explicitly querying for C++ front end.
>>>
>>> Johann
>>>
>>> --
>>>
>>>      PR target/34734
>>>      * config/avr/avr.c (avr_handle_progmem_attribute): Hack around
>>>      non-present DECL_INITIAL if front end is C++.
>
>
>

Re: [Patch, AVR]: Hack around PR34734

[Georg-Johann Lay]

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

Richard Guenther schrieb:
> On Wed, Jun 15, 2011 at 3:54 PM, Georg-Johann Lay <avr@...> wrote:

Is it possible not to quote my email address? Thanks.

>> Weddington, Eric schrieb:
>>> Hi Johann,
>>>
>>> I understand your reasoning, but I'm not particularly fond of this hack.
>> Yes, ACK. It's a hack to get rid of the PR.
>>
>>> Surely there's a way to fix this correctly without relying on this hack...
> 
> Well, doing this error handling at attribute parsing time is going to
> be fragile.
> I suggest to move it to code emission time instead (there currently isn't a
> target hook for when the frontend finished a variable declaration).
> 
> Richard.

Thanks, appears I was a bit lazy in the original patch...

So here it is, no more a hack :-)

	PR target/34734
	* config/avr/avr.c (avr_handle_progmem_attribute): Move warning
	about uninitialized data attributed 'progmem' from here...
	(avr_asm_named_section): ...to here.

I changed the warning message because we now actually have different
behavior: data will be put in progmem no matter if an initializer is
present or not, just a warning will pop up an case of missing
initializer and if Wuninitialized is on.

Johann

>>
>>> Eric Weddington
>>>
>>>> -----Original Message-----
>>>> From: Georg-Johann Lay [mailto:avr@...]

Ditto, quoting ones name here will suffice.

>>>> Sent: Wednesday, June 15, 2011 3:26 PM
>>>> To: gcc-patches@gcc.gnu.org
>>>> Cc: Weddington, Eric; Anatoly Sokolov; Denis Chertykov
>>>> Subject: [Patch, AVR]: Hack around PR34734
>>>>
>>>> PR34734 is an annoying, false C++ warning for code like
>>>>
>>>> const int x __attribute__((progmem)) = 1;
>>>>
>>>> progmem.c:1:30: warning: only initialized variables can be placed into
>>>> program memory area [enabled by default]
>>>>
>>>> The problem is that DECL_INITIAL is NULL at the specific point in
>>>> space and time (avr_handle_progmem_attribute) even though tree.def
>>>> promises otherwise.
>>>>
>>>> The patch hacks around by explicitly querying for C++ front end.
>>>>
>>>> Johann
>>>>
>>>>      PR target/34734
>>>>      * config/avr/avr.c (avr_handle_progmem_attribute): Hack around
>>>>      non-present DECL_INITIAL if front end is C++.


[-- Attachment #2: pr34734.diff --]
[-- Type: text/x-patch, Size: 1146 bytes --]

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(Revision 175036)
+++ config/avr/avr.c	(Arbeitskopie)
@@ -5099,12 +5099,7 @@ avr_handle_progmem_attribute (tree *node
 	}
       else if (TREE_STATIC (*node) || DECL_EXTERNAL (*node))
 	{
-	  if (DECL_INITIAL (*node) == NULL_TREE && !DECL_EXTERNAL (*node))
-	    {
-	      warning (0, "only initialized variables can be placed into "
-		       "program memory area");
-	      *no_add_attrs = true;
-	    }
+          *no_add_attrs = false;
 	}
       else
 	{
@@ -5293,6 +5288,15 @@ avr_asm_init_sections (void)
 void
 avr_asm_named_section (const char *name, unsigned int flags, tree decl)
 {
+  if (decl && DECL_P (decl)
+      && NULL_TREE == DECL_INITIAL (decl)
+      && avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
+    {
+      warning (OPT_Wuninitialized,
+               "uninitialized variable %q+D put into "
+               "program memory area", decl);
+    }
+
   if (!avr_need_copy_data_p)
     avr_need_copy_data_p = (0 == strncmp (name, ".data", 5)
                             || 0 == strncmp (name, ".rodata", 7)