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
