This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Postpone __LINE__ evaluation to the end of #line directives
- From: Marek Polacek <polacek at redhat dot com>
- To: mtewoodbury at gmail dot com
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 27 Nov 2013 11:46:33 +0100
- Subject: Re: [PATCH] Postpone __LINE__ evaluation to the end of #line directives
- Authentication-results: sourceware.org; auth=none
- References: <1385548162-1883-1-git-send-email-mtewoodbury at gmail dot com>
On Wed, Nov 27, 2013 at 05:29:22AM -0500, mtewoodbury@gmail.com wrote:
> From: Max TenEyck Woodbury <max+git@mtew.isa-geek.net>
This patch is badly missing a description. You also want to mention
the PR number, if this fixes a bug. I guess this is to fix PR58687.
> Copyright 2013 assigned to the Free Software Foundation.
> ---
> gcc/testsuite/gcc.dg/cpp/line4.c | 19 +++++++++++++++----
> libcpp/directives.c | 9 ++++++++-
> libcpp/internal.h | 1 +
> libcpp/macro.c | 3 +++
> 4 files changed, 27 insertions(+), 5 deletions(-)
You didn't attach a ChangeLog entry.
The patch has numerous formatting issues, see below.
> diff --git a/gcc/testsuite/gcc.dg/cpp/line4.c b/gcc/testsuite/gcc.dg/cpp/line4.c
> index 84dbf96..0120a2b 100644
> --- a/gcc/testsuite/gcc.dg/cpp/line4.c
> +++ b/gcc/testsuite/gcc.dg/cpp/line4.c
> @@ -13,7 +13,18 @@ enum { i = __LINE__ };
> enum { j = __LINE__ };
>
> #line 16 /* N.B. the _next_ line is line 16. */
> -
> -char array1[i == 44 ? 1 : -1];
> -char array2[j == 90 ? 1 : -1];
> -char array3[__LINE__ == 19 ? 1 : -1];
> + /* __LINE__ should be 16 */
> +char array1[i == 44 ? 1 : -1]; /* 17 */
> +char array2[j == 90 ? 1 : -1]; /* 18 */
> +char array3[__LINE__ == 19 ? 1 : -1]; /* 19 */
> + /* 20 */
> +# line __LINE__ /* N.B. the __LINE__ sequence should _not_ change here. */
Two spaces after '.'.
> +# line __LINE__ /* N.B. nor shoud a multi-line comment change the fact
"should"
> diff --git a/libcpp/directives.c b/libcpp/directives.c
> index 65b2034..adb04a5 100644
> --- a/libcpp/directives.c
> +++ b/libcpp/directives.c
> @@ -900,7 +900,9 @@ do_line (cpp_reader *pfile)
> bool wrapped;
>
> /* #line commands expand macros. */
> + ++pfile->state.in_directive; /* Request special __LINE__ handling. */
Don't put the comments next to the statements, instead put them above
the statements.
> token = cpp_get_token (pfile);
> + --pfile->state.in_directive; /* Cancle request */
"Cancel"
> - if (CPP_PEDANTIC (pfile) && (new_lineno == 0 || new_lineno > cap || wrapped))
> + if (CPP_PEDANTIC (pfile) && (new_lineno == 0 ||
> + (new_lineno > cap && new_lineno != CUR__LINE__) ||
> + wrapped))
|| operator shouldn't end the line, put it on the start of the next
line. See existing code for how it should look like.
> cpp_error (pfile, CPP_DL_PEDWARN, "line number out of range");
> else if (wrapped)
> cpp_error (pfile, CPP_DL_WARNING, "line number out of range");
> @@ -936,6 +940,9 @@ do_line (cpp_reader *pfile)
> }
>
> skip_rest_of_line (pfile);
> + if ( new_lineno == CUR__LINE__ ) /* Postponed evaluation ? */
> + new_lineno = linemap_get_expansion_line (pfile->line_table,
> + pfile->line_table->highest_line);
No space after '(' and before ')'. No space before '?'.
> +#define CUR__LINE__ -1U
The trailing underscores look weird to me. Thanks,
Marek