Re: including sparse headers in C++ code

[Christopher Li <sparse@xxxxxxxxxxx>]

On Tue, Oct 12, 2010 at 3:45 PM, Tomas Klacko <tomas.klacko@xxxxxxxxx> wrote:
> Hi,
>
> Thanks everyone for your comments, I have refined the patch
> and I am posting the next version (below).
>
> On Tue, Oct 12, 2010 at 1:01 AM, Christopher Li <sparse@xxxxxxxxxxx> wrote:
>> On Mon, Oct 11, 2010 at 3:46 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>>> On Tue, Oct 12, 2010 at 12:33:32AM +0200, Tomas Klacko wrote:
>>>

>>
>> I am about to complain the same thing. Al beats me to it.
>>
>> Tomas, can you limit the change on the header file C++ friendly first?
>> I don't think the rest of the C code need to be compile in C++, so it is fine
>> using C++ key words.
>
> I don't think I quite get your question. I have limited the amount of changes
> to the C source files, but there are still some. Which result for instance
> from renaming "struct symbol->namespace" to "struct symbol->ns".

Rename header members to avoid keyword are fine.  You don't need to change
any thing that is internal of a C code. e.g. from your previous patch:
<quote>
-static struct storage *get_reg(struct regclass *class)
+static struct storage *get_reg(struct regclass *Class)
</quote>

This function is  outside of header file.

> However, I can probably try doing:
> struct symbol
> {
> ...
> #ifndef __cplusplus
>  enum namespace namespace;
> #else
>  enum name_space ns;
> #endif

Even worse.

I don't like the #ifndef __cplusplus at all. Try to keep it minimal.
Just make it "enum name_space ns;" is clear better than maintain two
different names.

> and then try to work out any consequences solely in the header files.
> After all, using different names for the same types in C++ versus C
> should still result in correct code.

That is evil.

>        if (token->pos.type == TOKEN_IDENT) {
> +#ifndef __cplusplus
>                struct symbol *sym = lookup_symbol(token->ident, NS_SYMBOL | NS_TYPEDEF);
> -               return sym && (sym->namespace & NS_TYPEDEF);
> +#else
> +               struct symbol *sym = lookup_symbol(token->ident, (enum
> name_space)(NS_SYMBOL | NS_TYPEDEF));
> +#endif

Hmm, NS_SYMBOL | NS_TYPEDEF looks better.
Maybe declare NS_SYMBOL_OR_TYPEDEF = NS_SYMBOL | NS_TYPEDEF as
one of the enum name_space so we don't need to force cast here.


> +#ifdef __cplusplus
> +extern "C" {
> +#endif

Can you move this into your C++ code before including the sparse headers?
I don't want it in the sparse headers. Sparse is compiled in C,
shouldn't need to know
C++ at all.

>-       .class = CChar,
>+       .cls = CChar,

"klass" instead of "cls" ?

>
> +#ifdef __cplusplus
> +}
> +#endif
> +

And this too.

>  {
> +#ifndef __cplusplus
>        return undo_ptr_list_last((struct ptr_list **)head);
> +#else
> +       return (struct instruction *)undo_ptr_list_last((struct ptr_list **)head);
> +#endif

Just keep the else branch and remove the #ifndef. It is harmless to do
a type cast
in C here. Same for the following inline functions.


>  static inline void concat_symbol_list(struct symbol_list *from,
> struct symbol_list **to)
> diff --git a/linearize.h b/linearize.h
> index 50b3601..a385968 100644
> --- a/linearize.h
> +++ b/linearize.h
> @@ -314,9 +314,9 @@ static inline void remove_bb_from_list(struct
> basic_block_list **list, struct ba
>  }
>
>  static inline void replace_bb_in_list(struct basic_block_list **list,
> -       struct basic_block *old, struct basic_block *new, int count)
> +       struct basic_block *old, struct basic_block *new_list, int count)
>  {
> -       replace_ptr_list_entry((struct ptr_list **)list, old, new, count);
> +       replace_ptr_list_entry((struct ptr_list **)list, old, new_list, count);

new_list -> newlist

> diff --git a/parse.h b/parse.h
> index 6b21e23..f2193e7 100644
> --- a/parse.h
> +++ b/parse.h
> @@ -35,10 +35,12 @@ struct statement {
>                struct /* declaration */ {
>                        struct symbol_list *declaration;
>                };
> +#ifndef __cplusplus
>                struct /* label_arg */ {
>                        struct symbol *label;
>                        struct statement *label_statement;
>                };
> +#endif

What is this #ifndef  for?


>  /* Silly type-safety check ;) */
>  #define DECLARE_PTR_LIST(listname,type)        struct listname { type *list[1]; }
> -#define CHECK_TYPE(head,ptr)           (void)(&(ptr) == &(head)->list[0])
>  #define TYPEOF(head)                   __typeof__(&(head)->list[0])
>  #define VRFY_PTR_LIST(head)            (void)(sizeof((head)->list[0]))
>
> +#ifndef __cplusplus
> +#define CHECK_TYPE(head,ptr)           (void)(&(ptr) == &(head)->list[0])
> +#else
> +/* I don't know yet how to do this better in C++. */
> +#define CHECK_TYPE(head,ptr) (void)((void*)&(ptr) == (void*)&(head)->list[0])
> +#endif

If you can't get CHECK_TYPE work in C++, you might just make it an empty define
instead of doing useless point dancing. At least it is clear that it does not
do any thing here.

> +#ifdef __cplusplus
> +extern "C" {
> +#endif

Get rid of this too.

>  static inline void update_tag(void *p, unsigned long tag)
>  {
> +#ifndef __cplusplus
>        unsigned long *ptr = p;
> +#else
> +       unsigned long *ptr = (unsigned long *)p;
> +#endif
>        *ptr = tag | (~3UL & *ptr);
>  }

Just keep the #else branch and get rid of the #ifndef


> +++ b/token.h
> @@ -175,7 +175,11 @@ struct token {
>  static inline struct token *containing_token(struct token **p)
>  {
>        void *addr = (char *)p - ((char *)&((struct token *)0)->next - (char *)0);
> +#ifndef __cplusplus
>        return addr;
> +#else
> +       return (struct token*)addr;
> +#endif

Same here.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html