[Mesa-dev] [PATCH 02/12] android: define required __STDC* macros as cflags

[Mauro Rossi]

2017-04-30 11:43 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:

> Hi Rob,
>
> On 27 April 2017 at 20:43, Rob Herring <robh at kernel.org> wrote:
> > From: Mauro Rossi <issor.oruam at gmail.com>
> >
> > Necessary to fix the following radeonsi building errors:
> >
> > In file included from external/mesa/src/gallium/
> drivers/radeonsi/si_blit.c:24:
> > In file included from external/mesa/src/gallium/
> drivers/radeonsi/si_pipe.h:29:
> > In file included from external/mesa/src/gallium/
> drivers/radeonsi/si_shader.h:71:
> > In file included from external/llvm/include/llvm-c/Core.h:18:
> > In file included from external/llvm/include/llvm-c/ErrorHandling.h:17:
> > In file included from external/llvm/include/llvm-c/Types.h:17:
> > external/llvm/include/llvm/Support/DataTypes.h:49:3: error: "Must
> #define __STDC_LIMIT_MACROS before #including Support/DataTypes.h"
> >   ^
> > external/llvm/include/llvm/Support/DataTypes.h:53:3: error: "Must
> #define __STDC_CONSTANT_MACROS before "         "#including
> Support/DataTypes.h"
> >   ^
> > 2 errors generated.
>
> AFAICT this patch is a hack, as such can we try and fix it in the correct
> place?
> Alternatively please add a big note HACK with some information.
>
> More elaborate reply follows below, as posted on the Android-x86 mailing
> list.
>
> Thanks
> Emil
>
> "...one might
> require the defines _only_ when building C++ sources.
>
> Skimming through my local include/llvm/Support/DataTypes.h we have:
>
> ...
> #define HAVE_STDINT_H 1
> ...
>
> #ifdef __cplusplus
> #include <cstdint>
> #else
> #ifdef HAVE_STDINT_H
> #include <stdint.h>
> #else
> #error "Compiler must provide an implementation of stdint.h"
> #endif
> #endif
>
> #ifndef _MSC_VER
>
> #if !defined(UINT32_MAX)
> # error "The standard header <cstdint> is not C++11 compliant. Must
> #define "\
>        "__STDC_LIMIT_MACROS before #including Support/DataTypes.h"
> #endif
>
> #if !defined(UINT32_C)
> # error "The standard header <cstdint> is not C++11 compliant. Must
> #define "\
>         "__STDC_CONSTANT_MACROS before #including Support/DataTypes.h"
>
>
> In my header [snipped above] the error message is misleading/wrong. We
> are using cstdint only in C++ context, and stdint.h in C one. But
> that's nothing for you to worry.
>
> Yet both UINT32_MAX and UINT32_C are defined in stdint.h since they
> are part of the C99 standard, which we explicitly require. Even on
> Android via LOCAL_CONLYFLAGS += -std=c99.
>

external/libcxx/include/cstdint does not define UINT32_MAX nor UINT32_C
it relies on #include <stdint.h>

and in bionic's <stdint.h> I see:

#ifndef _STDINT_H
#define _STDINT_H

#include <bits/wchar_limits.h>
#include <stddef.h>
...
/* Keep the kernel from trying to define these types... */
#define __BIT_TYPES_DEFINED__
...
#define UINT32_C(c)       c ## U
...
#define UINT32_MAX       (4294967295U)
...
#endif /* _STDINT_H */




>
>
> Please address this in the correct place. Some how-to/directions:
>  - check if -std=c99 is correctly propagated
>  - check that the llvm header is sane (should look roughly like above)
>  - the stdint.h ends up included and the required macros are guarded
> appropriately - ideally there should be no guards, since C11 and C++11
> explicitly made the UINT32* macros "allowed".
> "
>

he-he, very funny, those macro are considered harmful ...and were revised
in several TC,
and they surely are if they still cause issues.

In the link [1] the problem with in llvm is known and acknowledged,
however they still check for UINT32_MAX UINT32_C being defined,
but are they?

<cstdlib> in libcxx/includes path just wraps bionic's <stdint.h> which has
the macros,

But if -std=c99 is used, is the followwing comment in
http://en.cppreference.com/w/cpp/types/integer to be taken in consideration?

<<The C99 standard suggests that C++ implementations should not define the
above limit, constant, or format macros unless the macros
__STDC_LIMIT_MACROS, __STDC_CONSTANT_MACROS or __STDC_FORMAT_MACROS
(respectively) are defined before including the relevant C header (stdint.h
or inttypes.h). This recommendation was not adopted by any C++ standard and
was removed in C11. However, some implementations (such as glibc 2.17) try
to apply this rule, and it may be necessary to define the __STDC macros;
C++ compilers may try to work around this by automatically defining them in
some circumstances.>>

And if the macro __STDC_LIMIT_MACROS are not necessarily to be defined in
C++11,

I can't see includes of <cstdint> or <stdint.h> in
src/gallium/drivers/radeonsi

utente at utente-System-Product-Name:~/nougat-x86/external/mesa/src/gallium/drivers/radeonsi$
cgrep cstdint
utente at utente-System-Product-Name:~/nougat-x86/external/mesa/src/gallium/drivers/radeonsi$
cgrep stdint
utente at utente-System-Product-Name
:~/nougat-x86/external/mesa/src/gallium/drivers/radeonsi$

So how will the UINT32_MAX,  UINT32_C macros be defined? Are they
pre-bundled?
I ask because I don't know

M.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170430/c56d19b6/attachment-0001.html>