RE: [avr-gcc-list] PSTR macro bug in C++

[Theodore A. Roth]

On Fri, 10 Sep 2004, Dave Hylands wrote:

> Hi Ted,
>
> I don't claim to totally understand the whole progmem thing, but I'm
> pretty good with C, so I'll make a couple of comments.
>
> >   #define PSTR(s) ({static char __c[] PROGMEM = (s); &(*__c);})
> >   #define PSTR(s) ({static char __c[] PROGMEM = (s); &__c[0];})
> >   #define PSTR(s) ({static char __c[] PROGMEM = (s); (PGM_P)__c;})
> >
> > The third one seems the most likely to be committed. Any C/C++ gurus
> > have anything to say on this before I close out the bug (I've been
> > sitting on this for way too long).
>
> PGM_P is #defined to be "const prog_char *" (I'm not sure why it's
> #defined rather than typedef'd, but that's another issue).
>
> So it seems to me that this should be changeable to this:
>
>       #define PSTR(s) ({static const char __c[] PROGMEM = (s); __c;})
>
> And I believe that this should also be equivalent
>
>       #define PSTR(s) ({static const prog_char __c[] = (s); __c;})

Even with the const it's still a problem for g++. I'm getting this error
with my test app:

  $ avr-g++ --version
  avr-g++ (GCC) 3.4.2 20040727 (prerelease)
  $ make
  avr-g++ -g -Os -Wall -mmcu=atmega128     -c -o tst.o tst.C
  tst.C: In function `int main()':
  tst.C:20: internal compiler error: in simplify_gen_subreg, at
simplify-rtx.c:3444
  Please submit a full bug report,
  with preprocessed source if appropriate.
  See <URL:http://gcc.gnu.org/bugs.html> for instructions.
  make: *** [tst.o] Error 1

On a whim, I tried this,

  #define PSTR(s) ({static char __c[] PROGMEM = (s); (const char *)__c;})
  #define PSTR(s) ({static char __c[] PROGMEM = (s); (char *)__c;})

and my test app compiled successfully in both cases.

Just for grins, I tried compiling my test app with the native g++. I got
this error

  $ g++ --version
  g++ (GCC) 3.3.4 (Debian 1:3.3.4-6sarge1)
  $ g++ -o ntst tst.C
  tst.C: In function `int main()':
  tst.C:20: error: invalid use of non-lvalue array

if I used what's in avr-libc currently (avr-g++ gave the internal
error). If I cast __c to (prog_char *) or even (char *), native g++
worked too.

For completeness, here's my test app:

      1 #include <stdio.h>
      2 #ifdef __AVR__
      3 #  warning "building for avr"
      4 #  include <avr/pgmspace.h>
      5 #else
      6 #  warning "building natively"
      7 #  define PROGMEM
      8 #  define fdevopen(a,b,c) stdout
      9 #  define fprintf_P fprintf
     10 #endif
     11
     12 #undef PSTR
     13 #define PSTR(s) ({static char __c[] PROGMEM = (s); (prog_char *)__c;})
     14
     15 int dummy_putchar (char c) { return 0; }
     16
     17 int main (void)
     18 {
     19     FILE *fp = fdevopen (dummy_putchar, NULL, 0);
     20     fprintf_P (fp, PSTR ("Hello World! %d\n"), 1);
     21     return 0;
     22 }

>
> Personally, I like to not have casts if I don't need them.

Agreed, but we already have a lot of cast voodoo embedded in avr-libc.

> The first 2 variants don't return const pointers, so there is a
> fundamental difference between them and the 3rd variant. I'm not
> familiar enough with the typical usage to know if returning a const
> pointer will introduce any new warnings that weren't there before. I
> think that it SHOULD return a const pointer, but I know lots of people
> don't use const properly, and since it didn't return a const pointer
> before, it could cause some warnings.
>
> For example, I believe that this code would have worked before:
>
>       prog_char *c = PSTR( "Test" );
>
> Where it really should have been written
>
>       const prog_char *c = PSTR( "Test" );
>
> So code that's written the first way will now give a compiler warning if
> the PSTR macro is changed to the 3rd variation (or either of the ways I
> proposed).

Well, the const usage is a separate issue.

It appears that we can fix the build for the C++ problem with a simple
cast to (prog_char *) so I think we can avoid the PGM_P const
incompatibility you are noting.

---
Ted Roth
PGP Key ID: 0x18F846E9
Jabber ID: address@hidden