From patchwork Mon Aug 11 14:18:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 378999 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id ECB32140141 for ; Tue, 12 Aug 2014 00:18:39 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=eJi06cJZHphmkwID qjn/6QzCWk/o1Ex6GI7JOepz3C20EV4OCnRINEq2Npq5yWKw3cgJZ2C3W4P+E2bf fmSBkZlc1Ku3/dxxoQ8suhS33eKI0Jw7ZZJGtTj/CPR43+JfMpVOB9IjET+6nNuB yEDdoMDazosgNyYkzDeAfVEDUWE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=pzZy1blZKm+C919UuQPqyh NERDk=; b=V4826A1D1CBi808ZMcgmpnMxiACrb/LxElfrWCzjKzaNyDrdFKLSgD RJ5ugc3hybRe40H33ioT5Gdp0FMpI9g8zAII/ZnxmLl/IEbzFzSuWZEheG6mebFZ HvPnocyt8gEKJCRCg5iAoifi7sD6sVpzUJ9EKtb+XdVI0sQeFFdnc= Received: (qmail 22568 invoked by alias); 11 Aug 2014 14:18:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 22554 invoked by uid 89); 11 Aug 2014 14:18:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 11 Aug 2014 14:18:27 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7BEIObL030547 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 11 Aug 2014 10:18:25 -0400 Received: from localhost (ovpn-116-30.ams2.redhat.com [10.36.116.30]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7BEILTo028361 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 11 Aug 2014 10:18:23 -0400 Received: by localhost (Postfix, from userid 1001) id 552D8F1D; Mon, 11 Aug 2014 16:18:18 +0200 (CEST) From: Dodji Seketeli To: GCC Patches Cc: Jason Merrill , Tom Tromey , polacek@redhat.com, Jeff Law Subject: Re: [PATCH] PR preprocessor/61817 - Fix location of tokens in built-in macro expansion list References: <86pph5qtug.fsf@redhat.com> X-URL: http://www.redhat.com Date: Mon, 11 Aug 2014 16:18:17 +0200 In-Reply-To: <86pph5qtug.fsf@redhat.com> (Dodji Seketeli's message of "Wed, 16 Jul 2014 16:08:39 +0200") Message-ID: <86a97baz4m.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Hello, I have initially sent a patch for this at https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01144.html. But then a patch for a similarly expressed issue https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01521.html (which I think is the same underlying problem) was committed. As the PR preprocessor/61817 is still not fixed, I have updated my initial patch over the one that was committed already and am submitted the result below. I have added more test cases in the process. Consider this function-like macro definition in the inc.h file ------------------->inc.h<------------------- #define F() const int line = __LINE__ ------------------->8<----------------------- Now consider its use in a file test.c: ------------------>cat -n test.c<---------------- 1 #include "inc.h" 2 3 void 4 foo() 5 { 6 F 7 ( 8 ) 9 ; 10 } ------------------->8<--------------------- Running test.c through cc1 -quiet -E yields: --------------------->8<-------------------- # 1 "test.c" # 1 "" # 1 "" # 1 "/usr/include/stdc-predef.h" 1 3 4 # 1 "" 2 # 1 "test.c" # 1 "inc.h" 1 # 2 "test.c" 2 void foo() { const int line = 8 ; } --------------------->8<---------------------- Note how tokens "const", "int", "line" and "=" are all on the same line (line 6) as the expansion point of the function-like F() macro, but how the token "8", resulting from the expansion of the built-in macro __FILE__ is on the same line as the closing parenthesis of the invocation of F(). This is the problem. The result of the __LINE__ macro should be "6" and should be on the same line (line 6) as the other tokens of the expansion-list of F(). This issue actually holds for the expansion of all built-in macros. So more generelly, I would describe the issue as such: When expanded in a function-like macro, the location of resulting tokens of a built-in macro is set to the closing parenthesis of the enclosing function-like macro invocation, rather than being set to the location of the expansion point of the invocation the enclosing functin-like macro. I think the problem is that enter_macro_context() is passed the location of the expansion point of the macro that is about to be expanded. But when that macro is built-in, builtin_macro() that is then called by enter_macro_context() doesn't use the macro expansion point location when expanding the __LINE__ builtin (in, e.g, _cpp_builtin_macro_text) . Rather, what it used is the location of the closing parenthesis of the enclosing function-like macro invocation or, more generally, the location of the previous token in the stream. The patch proposes to make builtin_macro() use the expansion point location that enter_macro_context() passed along. That location is then used by the different routines that need it (e.g, to expand builtin macros for instance). Also, for the particular case of __LINE__, make that built-in expand to the line number of that expansion point. Note that in the function builtin_macro, this patch avoids re-setting token->src_loc because for ease of maintenance (and debugging), I think it's preferable that the spelling location of tokens be set only in place -- in the tokens lexer. To control the value of that location we use the function cpp_force_token_locations, as intended. The patch bootstraps and passes tests on x86_64-unknown-linux-gnu against trunk. libcpp/ * macro.c (_cpp_builtin_macro_text): Honor the cpp_reader::forced_token_location_p data member to force the value __LINE__ expands to accordlingly. (builtin_macro): Use cpp_force_token_locations() to set the location of the resulting token to that expansion point location. (enter_macro_context): Pass the expansion point locatoin to builtin_macro. gcc/testsuite/ * gcc.dg/cpp/builtin-macro-{0,1,2,3}.c: New test files. Signed-off-by: Dodji Seketeli --- gcc/testsuite/gcc.dg/cpp/builtin-macro-0.c | 18 ++++++++++ gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c | 30 ++++++++++++++++ gcc/testsuite/gcc.dg/cpp/builtin-macro-2.c | 28 +++++++++++++++ gcc/testsuite/gcc.dg/cpp/builtin-macro-3.c | 28 +++++++++++++++ libcpp/macro.c | 56 +++++++++++++++++++++++------- 5 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/cpp/builtin-macro-0.c create mode 100644 gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c create mode 100644 gcc/testsuite/gcc.dg/cpp/builtin-macro-2.c create mode 100644 gcc/testsuite/gcc.dg/cpp/builtin-macro-3.c diff --git a/gcc/testsuite/gcc.dg/cpp/builtin-macro-0.c b/gcc/testsuite/gcc.dg/cpp/builtin-macro-0.c new file mode 100644 index 0000000..f57dd5b --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/builtin-macro-0.c @@ -0,0 +1,18 @@ +/* Origin: PR preprocessor/61817 + + In this test case, the diagnostic (happening for tokens resulting + from the expansion of the built-in token __FILE__) must point to the + expansion point of the enclosing function-like macro invocation. + + { dg-do compile } + { dg-options "-no-integrated-cpp" } */ + +#define F() static const char* __FILE__ + +void +foo () +{ + F /* { dg-error "expected identifier" } */ + ( + ); +} diff --git a/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c b/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c new file mode 100644 index 0000000..280797b --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c @@ -0,0 +1,30 @@ +/* Origin: PR preprocessor/61817 + + This test detects that the __LINE__ built-in macro expands to the + line number of the expansion point of the function-like macro in + which it's exanded. So if you change the content of this file, + please adjust the test to reflect the new lines layout. + + { dg-do run } + { dg-options -no-integrated-cpp } */ + +#include + +#define F() static const int line = __LINE__; + +void +foo () +{ + F /* the__LINE__ macro should expand to this line number. */ + ( + ); + + assert (line == 18); +} + +int +main () +{ + foo(); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/cpp/builtin-macro-2.c b/gcc/testsuite/gcc.dg/cpp/builtin-macro-2.c new file mode 100644 index 0000000..ffbeb75 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/builtin-macro-2.c @@ -0,0 +1,28 @@ +/* Origin: PR preprocessor/61817 + + This test detects that the __LINE__ built-in macro expands to the + line number of the expansion point of the function-like macro that + initially triggered the expansion stack. + + { dg-do compile } + { dg-options -no-integrated-cpp } */ + +#define JOIN(a,b) DO_JOIN(a,b) +#define DO_JOIN(a,b) DO_JOIN2(a,b) +#define DO_JOIN2(a,b) a##b +#define STATIC_ASSERT(expr) \ + int JOIN(a,__LINE__)[expr? 1 : -1] /* __LINE__ should not expand to + this line. */ + +void +foo() +{ + STATIC_ASSERT(1); + STATIC_ASSERT(2); /* If __LINE__ (wrongly) expands to + the point where it's used in the + definition of JOIN above, this + statement would trigger an error + about an already defined + variable. named + 'a'. */ +} diff --git a/gcc/testsuite/gcc.dg/cpp/builtin-macro-3.c b/gcc/testsuite/gcc.dg/cpp/builtin-macro-3.c new file mode 100644 index 0000000..53b698c --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/builtin-macro-3.c @@ -0,0 +1,28 @@ +/* Origin: PR preprocessor/61817 + + This test detects that the __LINE__ built-in macro expands to the + line number of the expansion point of the function-like macro that + initially triggered the expansion stack. + + { dg-do compile } + { dg-options -ftrack-macro-expansion=0 } */ + +#define JOIN(a,b) DO_JOIN(a,b) +#define DO_JOIN(a,b) DO_JOIN2(a,b) +#define DO_JOIN2(a,b) a##b +#define STATIC_ASSERT(expr) \ + int JOIN(a,__LINE__)[expr? 1 : -1] /* __LINE__ should not expand to + this line. */ + +void +foo() +{ + STATIC_ASSERT(1); + STATIC_ASSERT(2); /* If line (wrongly) expands to the + point where it's used in the + definition of JOIN above, this + statement would trigger an error + about an already defined + variable. named + 'a'. */ +} diff --git a/libcpp/macro.c b/libcpp/macro.c index ff6685c..20f092d 100644 --- a/libcpp/macro.c +++ b/libcpp/macro.c @@ -309,10 +309,26 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node) /* If __LINE__ is embedded in a macro, it must expand to the line of the macro's invocation, not its definition. Otherwise things like assert() will not work properly. */ - number = linemap_get_expansion_line (pfile->line_table, - CPP_OPTION (pfile, traditional) - ? pfile->line_table->highest_line - : pfile->cur_token[-1].src_loc); + if (pfile->forced_token_location_p) + { + /* We were asked to force the location of the resulting + token to have a certain value. + + If we are tracking macro locations, this location might be + virtual. In that case, resolve it to the root expansion + point of the macro that triggered the whole expansion + sequence. */ + number = linemap_resolve_location (pfile->line_table, + *pfile->forced_token_location_p, + LRK_MACRO_EXPANSION_POINT, + NULL); + number = linemap_get_expansion_line (pfile->line_table, number); + } + else + number = linemap_get_expansion_line (pfile->line_table, + CPP_OPTION (pfile, traditional) + ? pfile->line_table->highest_line + : pfile->cur_token[-1].src_loc); break; /* __STDC__ has the value 1 under normal circumstances. @@ -398,11 +414,14 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node) /* Convert builtin macros like __FILE__ to a token and push it on the context stack. Also handles _Pragma, for which a new token may not - be created. Returns 1 if it generates a new token context, 0 to - return the token to the caller. LOC is the location of the expansion - point of the macro. */ + be created. NODE is the built-in macro to consider. + EXPANSION_LOCATION is the location of the expansion of the macro + NODE. Note that this location can be virtual if we are tracking + macro locations. Returns 1 if it generates a new token context, 0 + to return the token to the caller. */ static int -builtin_macro (cpp_reader *pfile, cpp_hashnode *node, source_location loc) +builtin_macro (cpp_reader *pfile, cpp_hashnode *node, + source_location expansion_location) { const uchar *buf; size_t len; @@ -418,6 +437,13 @@ builtin_macro (cpp_reader *pfile, cpp_hashnode *node, source_location loc) return _cpp_do__Pragma (pfile); } + /* Make sure the location of the token resulting from the expansion + of the built-in token is the location of the expansion point that + we are passed. As a result, _cpp_builtin_macro_text() and + _cpp_lex_direct() that are called below will set the location of + their resulting token to this expansion point. */ + cpp_force_token_locations (pfile, &expansion_location); + buf = _cpp_builtin_macro_text (pfile, node); len = ustrlen (buf); nbuf = (char *) alloca (len + 1); @@ -430,8 +456,8 @@ builtin_macro (cpp_reader *pfile, cpp_hashnode *node, source_location loc) /* Set pfile->cur_token as required by _cpp_lex_direct. */ pfile->cur_token = _cpp_temp_token (pfile); cpp_token *token = _cpp_lex_direct (pfile); - /* We should point to the expansion point of the builtin macro. */ - token->src_loc = loc; + cpp_stop_forcing_token_locations (pfile); + if (pfile->context->tokens_kind == TOKENS_KIND_EXTENDED) { /* We are tracking tokens resulting from macro expansion. @@ -442,7 +468,7 @@ builtin_macro (cpp_reader *pfile, cpp_hashnode *node, source_location loc) _cpp_buff *token_buf = tokens_buff_new (pfile, 1, &virt_locs); const line_map * map = linemap_enter_macro (pfile->line_table, node, - token->src_loc, 1); + expansion_location, 1); tokens_buff_add_token (token_buf, virt_locs, token, pfile->line_table->builtin_location, pfile->line_table->builtin_location, @@ -1215,7 +1241,13 @@ enter_macro_context (cpp_reader *pfile, cpp_hashnode *node, pfile->about_to_expand_macro_p = false; /* Handle built-in macros and the _Pragma operator. */ - return builtin_macro (pfile, node, location); + { + source_location root_expansion_location = + CPP_OPTION (pfile, track_macro_expansion) + ? location + : pfile->invocation_location; + return builtin_macro (pfile, node, root_expansion_location); + } } /* De-allocate the memory used by BUFF which is an array of instances