Re: [PATCH] bpf: accept # as an inline comment char

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

> On 7/25/23 15:58, Jose E. Marchesi wrote:
>> 
>>> On 7/25/23 13:56, Jose E. Marchesi wrote:
>>>>
>>>>> This little patch makes the BPF assembler accept '#' as an inline
>>>>> comment character, which clang -S seems to use.
>>>>>
>>>>> Tested on bpf-unknown-none.
>>>>> OK?
>>>>>
>>>>> gas/
>>>>> 	* config/tc-bpf.c (comment_chars): Add '#'.
>>>>> 	* doc/c-bpf.texi (BPF Special Characters): Add note that '#' may
>>>>> 	be used for inline comments.
>>>>> ---
>>>>>  gas/config/tc-bpf.c | 2 +-
>>>>>  gas/doc/c-bpf.texi  | 7 ++++---
>>>>>  2 files changed, 5 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
>>>>> index c56b8fe984f..56c434c223d 100644
>>>>> --- a/gas/config/tc-bpf.c
>>>>> +++ b/gas/config/tc-bpf.c
>>>>> @@ -53,7 +53,7 @@ struct bpf_insn
>>>>>    unsigned int has_imm64 : 1;
>>>>>  };
>>>>>  
>>>>> -const char comment_chars[]        = ";";
>>>>> +const char comment_chars[]        = ";#";
>>>>>  const char line_comment_chars[]   = "#";
>>>>>  const char line_separator_chars[] = "`";
>>>>>  const char EXP_CHARS[]            = "eE";
>>>>> diff --git a/gas/doc/c-bpf.texi b/gas/doc/c-bpf.texi
>>>>> index 8f39ab314a7..597b3b30b30 100644
>>>>> --- a/gas/doc/c-bpf.texi
>>>>> +++ b/gas/doc/c-bpf.texi
>>>>> @@ -64,9 +64,10 @@ the host endianness is used.
>>>>>  
>>>>>  @cindex line comment character, BPF
>>>>>  @cindex BPF line comment character
>>>>> -The presence of a @samp{;} on a line indicates the start of a comment
>>>>> -that extends to the end of the current line.  If a @samp{#} appears as
>>>>> -the first character of a line, the whole line is treated as a comment.
>>>>> +The presence of a @samp{;} or a @samp{#} on a line indicates the start
>>>>> +of a comment that extends to the end of the current line.  If a
>>>>> +@samp{#} appears as the first character of a line, the whole line is
>>>>> +treated as a comment.
>>>>
>>>> Hm, isnt' this second sentence a quite obvious corollary now? :)
>>>
>>> Well, yes I suppose so.. :)
>>>
>>> I left it in because I did not touch (nor look further at):
>>>
>>>   const char line_comment_chars[]   = "#";
>>>
>>> which was what the sentence documented.
>>> I agree it doesn't add much anymore though, shall I remove it?
>> 
>> Hm, not sure.  Try if it makes a difference in practice...
>
> Yeah, it causes build failures if totally removed and failures
> in gas/macros/macros.exp if left blank.  Best to leave it.
>
> Here's a v2 with just the redundant sentence in the manual removed.

OK for master.
Thanks!


> This little patch makes the BPF assembler accept '#' as an inline
> comment character, which clang -S seems to use.
>
> gas/
> 	* config/tc-bpf.c (comment_chars): Add '#'.
> 	* doc/c-bpf.texi (BPF Special Characters): Add note that '#' may
> 	be used for inline comments.
> ---
>  gas/config/tc-bpf.c | 2 +-
>  gas/doc/c-bpf.texi  | 5 ++---
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
> index c56b8fe984f..56c434c223d 100644
> --- a/gas/config/tc-bpf.c
> +++ b/gas/config/tc-bpf.c
> @@ -53,7 +53,7 @@ struct bpf_insn
>    unsigned int has_imm64 : 1;
>  };
>  
> -const char comment_chars[]        = ";";
> +const char comment_chars[]        = ";#";
>  const char line_comment_chars[]   = "#";
>  const char line_separator_chars[] = "`";
>  const char EXP_CHARS[]            = "eE";
> diff --git a/gas/doc/c-bpf.texi b/gas/doc/c-bpf.texi
> index 8f39ab314a7..3fde0e6b613 100644
> --- a/gas/doc/c-bpf.texi
> +++ b/gas/doc/c-bpf.texi
> @@ -64,9 +64,8 @@ the host endianness is used.
>  
>  @cindex line comment character, BPF
>  @cindex BPF line comment character
> -The presence of a @samp{;} on a line indicates the start of a comment
> -that extends to the end of the current line.  If a @samp{#} appears as
> -the first character of a line, the whole line is treated as a comment.
> +The presence of a @samp{;} or a @samp{#} on a line indicates the start
> +of a comment that extends to the end of the current line.
>  
>  @cindex statement separator, BPF
>  Statements and assembly directives are separated by newlines.