This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Postpone __LINE__ evaluation to the end of #line directives


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]