Question on adding an option control to libcpp

Question on adding an option control to libcpp

[Qing Zhao]

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
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.

thanks a lot for the help.

Qing

Re: Question on adding an option control to libcpp

[David Malcolm]

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

Re: Question on adding an option control to libcpp

[Qing Zhao]

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
> 

Re: Question on adding an option control to libcpp

[Qing Zhao]

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

Re: Question on adding an option control to libcpp

[David Malcolm]

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
> 
> 

Re: Question on adding an option control to libcpp

[Qing Zhao]

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
>> 
>>