Question on adding an option control to libcpp

Message ID EB6EE95B-A124-4BD4-B743-A882BAA3DA3E@oracle.com
State New
Headers show
Series
  • Question on adding an option control to libcpp
Related show

Commit Message

Qing Zhao May 24, 2019, 4:59 p.m.
Hi, 

in order to fix PR90581: (provide an option to adjust the maximum depth of nested #include)

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581>

we need to add a new option to preprocessor.  where should I put this option?

I tried to add a new option into:

gcc/c-family/c.opt

[qinzhao@localhost c-family]$ git diff c.opt
thanks a lot for the help.

Qing

Comments

David Malcolm May 24, 2019, 6:47 p.m. | #1
On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:
> Hi, 

> 

> in order to fix PR90581: (provide an option to adjust the maximum

> depth of nested #include)

> 

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 <https://gcc.gnu.o

> rg/bugzilla/show_bug.cgi?id=90581>

> 

> we need to add a new option to preprocessor.  where should I put this

> option?

> 

> I tried to add a new option into:

> 

> gcc/c-family/c.opt

> 

> [qinzhao@localhost c-family]$ git diff c.opt

> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt

> index 046d489..4f237b6 100644

> --- a/gcc/c-family/c.opt

> +++ b/gcc/c-family/c.opt

> @@ -1598,6 +1598,10 @@ flocal-ivars

>  ObjC ObjC++ Var(flag_local_ivars) Init(1)

>  Allow access to instance variables as if they were local

> declarations within instance method implementati

>  

> +finclude-nest-limit=

> +C ObjC C++ ObjC++ Joined RejectNegative UInteger

> Var(include_nest_limit) Init(200)

> +Set the maximum number of depth of nested #include.

> +

> 

> However, don’t know how to refer this new variable

> “include_nest_limit” from libcpp.

> 


You probably want to add a new field to cpp_options, and copy over the
value from the GCC options to the new field; see e.g.
c_common_handle_option where various cases write to fields of cpp_opts.

Hope this is helpful
Dave
Qing Zhao May 24, 2019, 8:18 p.m. | #2
Hi, David,

this is helpful.

thanks.

Qing
> On May 24, 2019, at 1:47 PM, David Malcolm <dmalcolm@redhat.com> wrote:

> 

> On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:

>> Hi, 

>> 

>> in order to fix PR90581: (provide an option to adjust the maximum

>> depth of nested #include)

>> 

>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 <https://gcc.gnu.o

>> rg/bugzilla/show_bug.cgi?id=90581>

>> 

>> we need to add a new option to preprocessor.  where should I put this

>> option?

>> 

>> I tried to add a new option into:

>> 

>> gcc/c-family/c.opt

>> 

>> [qinzhao@localhost c-family]$ git diff c.opt

>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt

>> index 046d489..4f237b6 100644

>> --- a/gcc/c-family/c.opt

>> +++ b/gcc/c-family/c.opt

>> @@ -1598,6 +1598,10 @@ flocal-ivars

>> ObjC ObjC++ Var(flag_local_ivars) Init(1)

>> Allow access to instance variables as if they were local

>> declarations within instance method implementati

>> 

>> +finclude-nest-limit=

>> +C ObjC C++ ObjC++ Joined RejectNegative UInteger

>> Var(include_nest_limit) Init(200)

>> +Set the maximum number of depth of nested #include.

>> +

>> 

>> However, don’t know how to refer this new variable

>> “include_nest_limit” from libcpp.

>> 

> 

> You probably want to add a new field to cpp_options, and copy over the

> value from the GCC options to the new field; see e.g.

> c_common_handle_option where various cases write to fields of cpp_opts.

> 

> Hope this is helpful

> Dave

>
Qing Zhao May 28, 2019, 8:08 p.m. | #3
Hi, David,

for this new option, do I need to add a new testing case for it?
if so, where should I put this new testing case?

thanks.

Qing
> On May 24, 2019, at 1:47 PM, David Malcolm <dmalcolm@redhat.com> wrote:

> 

> On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:

>> Hi, 

>> 

>> in order to fix PR90581: (provide an option to adjust the maximum

>> depth of nested #include)

>> 

>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 <https://gcc.gnu.o

>> rg/bugzilla/show_bug.cgi?id=90581>

>> 

>> we need to add a new option to preprocessor.  where should I put this

>> option?

>> 

>> I tried to add a new option into:

>> 

>> gcc/c-family/c.opt

>> 

>> [qinzhao@localhost c-family]$ git diff c.opt

>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt

>> index 046d489..4f237b6 100644

>> --- a/gcc/c-family/c.opt

>> +++ b/gcc/c-family/c.opt

>> @@ -1598,6 +1598,10 @@ flocal-ivars

>> ObjC ObjC++ Var(flag_local_ivars) Init(1)

>> Allow access to instance variables as if they were local

>> declarations within instance method implementati

>> 

>> +finclude-nest-limit=

>> +C ObjC C++ ObjC++ Joined RejectNegative UInteger

>> Var(include_nest_limit) Init(200)

>> +Set the maximum number of depth of nested #include.

>> +

>> 

>> However, don’t know how to refer this new variable

>> “include_nest_limit” from libcpp.

>> 

> 

> You probably want to add a new field to cpp_options, and copy over the

> value from the GCC options to the new field; see e.g.

> c_common_handle_option where various cases write to fields of cpp_opts.

> 

> Hope this is helpful

> Dave
David Malcolm May 28, 2019, 9:26 p.m. | #4
On Tue, 2019-05-28 at 15:08 -0500, Qing Zhao wrote:
> Hi, David,

> 

> for this new option, do I need to add a new testing case for it?


Yes please.  Ideally we should add at least one test case for any new
option, especially if it's easy to test for.

> if so, where should I put this new testing case?


It's probably testable for in DejaGnu with a testcase that sets the
maximum depth to some very low limit (say just a single include), and
then have a testcase that has a depth of two.

Or something like that, though DejaGnu's line-based directives aren't a
perfect fit for diagnostics occurring in included files.

Maybe such tests should live in gcc/testsuite/c-c++-common/cpp/

I think there's something similar in:
  gcc/testsuite/c-c++-common/inc-from-1.c

Dave

> thanks.

> 

> Qing

> > On May 24, 2019, at 1:47 PM, David Malcolm <dmalcolm@redhat.com>

> > wrote:

> > 

> > On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:

> > > Hi, 

> > > 

> > > in order to fix PR90581: (provide an option to adjust the maximum

> > > depth of nested #include)

> > > 

> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 <https://gcc.g

> > > nu.o

> > > rg/bugzilla/show_bug.cgi?id=90581>

> > > 

> > > we need to add a new option to preprocessor.  where should I put

> > > this

> > > option?

> > > 

> > > I tried to add a new option into:

> > > 

> > > gcc/c-family/c.opt

> > > 

> > > [qinzhao@localhost c-family]$ git diff c.opt

> > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt

> > > index 046d489..4f237b6 100644

> > > --- a/gcc/c-family/c.opt

> > > +++ b/gcc/c-family/c.opt

> > > @@ -1598,6 +1598,10 @@ flocal-ivars

> > > ObjC ObjC++ Var(flag_local_ivars) Init(1)

> > > Allow access to instance variables as if they were local

> > > declarations within instance method implementati

> > > 

> > > +finclude-nest-limit=

> > > +C ObjC C++ ObjC++ Joined RejectNegative UInteger

> > > Var(include_nest_limit) Init(200)

> > > +Set the maximum number of depth of nested #include.

> > > +

> > > 

> > > However, don’t know how to refer this new variable

> > > “include_nest_limit” from libcpp.

> > > 

> > 

> > You probably want to add a new field to cpp_options, and copy over

> > the

> > value from the GCC options to the new field; see e.g.

> > c_common_handle_option where various cases write to fields of

> > cpp_opts.

> > 

> > Hope this is helpful

> > Dave

> 

>
Qing Zhao May 28, 2019, 9:29 p.m. | #5
Okay.

thanks, I will add testing cases.

Qing
> On May 28, 2019, at 4:26 PM, David Malcolm <dmalcolm@redhat.com> wrote:

> 

> On Tue, 2019-05-28 at 15:08 -0500, Qing Zhao wrote:

>> Hi, David,

>> 

>> for this new option, do I need to add a new testing case for it?

> 

> Yes please.  Ideally we should add at least one test case for any new

> option, especially if it's easy to test for.

> 

>> if so, where should I put this new testing case?

> 

> It's probably testable for in DejaGnu with a testcase that sets the

> maximum depth to some very low limit (say just a single include), and

> then have a testcase that has a depth of two.

> 

> Or something like that, though DejaGnu's line-based directives aren't a

> perfect fit for diagnostics occurring in included files.

> 

> Maybe such tests should live in gcc/testsuite/c-c++-common/cpp/

> 

> I think there's something similar in:

>  gcc/testsuite/c-c++-common/inc-from-1.c

> 

> Dave

> 

>> thanks.

>> 

>> Qing

>>> On May 24, 2019, at 1:47 PM, David Malcolm <dmalcolm@redhat.com>

>>> wrote:

>>> 

>>> On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:

>>>> Hi, 

>>>> 

>>>> in order to fix PR90581: (provide an option to adjust the maximum

>>>> depth of nested #include)

>>>> 

>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 <https://gcc.g

>>>> nu.o

>>>> rg/bugzilla/show_bug.cgi?id=90581>

>>>> 

>>>> we need to add a new option to preprocessor.  where should I put

>>>> this

>>>> option?

>>>> 

>>>> I tried to add a new option into:

>>>> 

>>>> gcc/c-family/c.opt

>>>> 

>>>> [qinzhao@localhost c-family]$ git diff c.opt

>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt

>>>> index 046d489..4f237b6 100644

>>>> --- a/gcc/c-family/c.opt

>>>> +++ b/gcc/c-family/c.opt

>>>> @@ -1598,6 +1598,10 @@ flocal-ivars

>>>> ObjC ObjC++ Var(flag_local_ivars) Init(1)

>>>> Allow access to instance variables as if they were local

>>>> declarations within instance method implementati

>>>> 

>>>> +finclude-nest-limit=

>>>> +C ObjC C++ ObjC++ Joined RejectNegative UInteger

>>>> Var(include_nest_limit) Init(200)

>>>> +Set the maximum number of depth of nested #include.

>>>> +

>>>> 

>>>> However, don’t know how to refer this new variable

>>>> “include_nest_limit” from libcpp.

>>>> 

>>> 

>>> You probably want to add a new field to cpp_options, and copy over

>>> the

>>> value from the GCC options to the new field; see e.g.

>>> c_common_handle_option where various cases write to fields of

>>> cpp_opts.

>>> 

>>> Hope this is helpful

>>> Dave

>> 

>>

Patch

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 046d489..4f237b6 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1598,6 +1598,10 @@  flocal-ivars
 ObjC ObjC++ Var(flag_local_ivars) Init(1)
 Allow access to instance variables as if they were local declarations within instance method implementati
 
+finclude-nest-limit=
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(include_nest_limit) Init(200)
+Set the maximum number of depth of nested #include.
+

However, don’t know how to refer this new variable “include_nest_limit” from libcpp.