Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

study /foo/ should warn #12494

Open
p5pRT opened this issue Oct 14, 2012 · 11 comments
Open

study /foo/ should warn #12494

p5pRT opened this issue Oct 14, 2012 · 11 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 14, 2012

Migrated from rt.perl.org#115264 (status was 'open')

Searchable as RT115264$

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2012

From @cpansprout

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

  Jumbo regexp patch applied (with minor fix-up tweaks)​:
  Subject​: Version 7 of Jumbo RE patch available
 
  p4raw-id​: //depot/perl@​267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
  case '/'​: /* may either be division or pattern */
  case '?'​: /* may either be conditional or pattern */
  if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) || isALNUM(last_uni[5])))
+ check_uni();
  s = scan_pat(s);
  TERM(sublex_start());
  }

Studying the boolean returned by /.../ is not helpful, as no match is likely to be performed on that boolean. And if it were, study() would not help, as it has only ever helped for very long strings.


Flags​:
  category=core
  severity=low


Site configuration information for perl 5.17.5​:

Configured by sprout at Sat Sep 22 18​:51​:23 PDT 2012.

Summary of my perl5 (revision 5 version 17 subversion 5) configuration​:
  Snapshot of​: 451f421
  Platform​:
  osname=darwin, osvers=10.5.0, archname=darwin-2level
  uname='darwin pint.local 10.5.0 darwin kernel version 10.5.0​: fri nov 5 23​:20​:39 pdt 2010; root​:xnu-1504.9.17~1release_i386 i386 '
  config_args='-de -Dusedevel -DDEBUGGING'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=undef, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include',
  optimize='-O3 -g',
  cppflags='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.2.1 (Apple Inc. build 5664)', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib
  libs=-ldbm -ldl -lm -lutil -lc
  perllibs=-ldl -lm -lutil -lc
  libc=, so=dylib, useshrplib=false, libperl=libperl.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 


@​INC for perl 5.17.5​:
  /usr/local/lib/perl5/site_perl/5.17.5/darwin-2level
  /usr/local/lib/perl5/site_perl/5.17.5
  /usr/local/lib/perl5/5.17.5/darwin-2level
  /usr/local/lib/perl5/5.17.5
  /usr/local/lib/perl5/site_perl
  .


Environment for perl 5.17.5​:
  DYLD_LIBRARY_PATH (unset)
  HOME=/Users/sprout
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2012

From @demerphq

On 14 October 2012 23​:52, Father Chrysostomos <perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #115264]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115264 >

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

Jumbo regexp patch applied \(with minor fix\-up tweaks\)&#8203;:
    Subject&#8203;: Version 7 of Jumbo RE patch available

p4raw\-id&#8203;: //depot/perl@&#8203;267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
case '/'​: /* may either be division or pattern */
case '?'​: /* may either be conditional or pattern */
if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) || isALNUM(last_uni[5])))
+ check_uni();
s = scan_pat(s);
TERM(sublex_start());
}

Studying the boolean returned by /.../ is not helpful, as no match is likely to be performed on that boolean. And if it were, study() would not help, as it has only ever helped for very long strings.

I thought we made study a noop some time back? If we bring it back
maybe they will optimize this.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2012

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2012

From @cpansprout

On Mon Oct 15 02​:54​:07 2012, demerphq wrote​:

On 14 October 2012 23​:52, Father Chrysostomos <perlbug-
followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #115264]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115264 >

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

Jumbo regexp patch applied \(with minor fix\-up tweaks\)&#8203;:
    Subject&#8203;: Version 7 of Jumbo RE patch available

p4raw\-id&#8203;: //depot/perl@&#8203;267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
case '/'​: /* may either be division or pattern
*/
case '?'​: /* may either be conditional or
pattern */
if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) ||
isALNUM(last_uni[5])))
+ check_uni();
s = scan_pat(s);
TERM(sublex_start());
}

Studying the boolean returned by /.../ is not helpful, as no match
is likely to be performed on that boolean. And if it were, study()
would not help, as it has only ever helped for very long strings.

I thought we made study a noop some time back? If we bring it back
maybe they will optimize this.

Studying &PL_sv_yes and &PL_sv_no?

(Sorry for the cryptic response, but I don’t know how to explain it
better than I did above.)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2012

From [Unknown Contact. See original ticket]

On Mon Oct 15 02​:54​:07 2012, demerphq wrote​:

On 14 October 2012 23​:52, Father Chrysostomos <perlbug-
followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #115264]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115264 >

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

Jumbo regexp patch applied \(with minor fix\-up tweaks\)&#8203;:
    Subject&#8203;: Version 7 of Jumbo RE patch available

p4raw\-id&#8203;: //depot/perl@&#8203;267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
case '/'​: /* may either be division or pattern
*/
case '?'​: /* may either be conditional or
pattern */
if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) ||
isALNUM(last_uni[5])))
+ check_uni();
s = scan_pat(s);
TERM(sublex_start());
}

Studying the boolean returned by /.../ is not helpful, as no match
is likely to be performed on that boolean. And if it were, study()
would not help, as it has only ever helped for very long strings.

I thought we made study a noop some time back? If we bring it back
maybe they will optimize this.

Studying &PL_sv_yes and &PL_sv_no?

(Sorry for the cryptic response, but I don’t know how to explain it
better than I did above.)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2012

From @demerphq

On 15 October 2012 16​:47, Father Chrysostomos via RT
<perlbug-comment@​perl.org> wrote​:

On Mon Oct 15 02​:54​:07 2012, demerphq wrote​:

On 14 October 2012 23​:52, Father Chrysostomos <perlbug-
followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #115264]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115264 >

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

Jumbo regexp patch applied \(with minor fix\-up tweaks\)&#8203;:
    Subject&#8203;: Version 7 of Jumbo RE patch available

p4raw\-id&#8203;: //depot/perl@&#8203;267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
case '/'​: /* may either be division or pattern
*/
case '?'​: /* may either be conditional or
pattern */
if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) ||
isALNUM(last_uni[5])))
+ check_uni();
s = scan_pat(s);
TERM(sublex_start());
}

Studying the boolean returned by /.../ is not helpful, as no match
is likely to be performed on that boolean. And if it were, study()
would not help, as it has only ever helped for very long strings.

I thought we made study a noop some time back? If we bring it back
maybe they will optimize this.

Studying &PL_sv_yes and &PL_sv_no?

(Sorry for the cryptic response, but I don’t know how to explain it
better than I did above.)

I don't find it cryptic so no worries. And yes while it is silly to
study these values IMO it makes no sense to warn about a no-op. Others
might disagree with me tho.

Im inclined to say that we should just remove *any* study based code
as it has been a no-op long enough that if it made a difference we
would have heard.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2012

From @cpansprout

On Mon Oct 15 07​:51​:23 2012, demerphq wrote​:

On 15 October 2012 16​:47, Father Chrysostomos via RT
<perlbug-comment@​perl.org> wrote​:

On Mon Oct 15 02​:54​:07 2012, demerphq wrote​:

On 14 October 2012 23​:52, Father Chrysostomos <perlbug-
followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #115264]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115264 >

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

Jumbo regexp patch applied \(with minor fix\-up tweaks\)&#8203;:
    Subject&#8203;: Version 7 of Jumbo RE patch available

p4raw\-id&#8203;: //depot/perl@&#8203;267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
case '/'​: /* may either be division or pattern
*/
case '?'​: /* may either be conditional or
pattern */
if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) ||
isALNUM(last_uni[5])))
+ check_uni();
s = scan_pat(s);
TERM(sublex_start());
}

Studying the boolean returned by /.../ is not helpful, as no match
is likely to be performed on that boolean. And if it were, study()
would not help, as it has only ever helped for very long strings.

I thought we made study a noop some time back? If we bring it back
maybe they will optimize this.

Studying &PL_sv_yes and &PL_sv_no?

(Sorry for the cryptic response, but I don’t know how to explain it
better than I did above.)

I don't find it cryptic so no worries. And yes while it is silly to
study these values IMO it makes no sense to warn about a no-op. Others
might disagree with me tho.

Aha, now I see what I forgot to say.

chr /foo/ warns about ambiguity. So does every other operator with a ;$
prototype, *except* study. Why do we need a special exception for study?

Im inclined to say that we should just remove *any* study based code
as it has been a no-op long enough that if it made a difference we
would have heard.

I have already removed it. :-)

Don’t remove study itself, though. I use it all the time so I can set a
breakpoint on Perl_pp_study in a C debugger. :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2012

From [Unknown Contact. See original ticket]

On Mon Oct 15 07​:51​:23 2012, demerphq wrote​:

On 15 October 2012 16​:47, Father Chrysostomos via RT
<perlbug-comment@​perl.org> wrote​:

On Mon Oct 15 02​:54​:07 2012, demerphq wrote​:

On 14 October 2012 23​:52, Father Chrysostomos <perlbug-
followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #115264]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115264 >

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

Jumbo regexp patch applied \(with minor fix\-up tweaks\)&#8203;:
    Subject&#8203;: Version 7 of Jumbo RE patch available

p4raw\-id&#8203;: //depot/perl@&#8203;267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
case '/'​: /* may either be division or pattern
*/
case '?'​: /* may either be conditional or
pattern */
if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) ||
isALNUM(last_uni[5])))
+ check_uni();
s = scan_pat(s);
TERM(sublex_start());
}

Studying the boolean returned by /.../ is not helpful, as no match
is likely to be performed on that boolean. And if it were, study()
would not help, as it has only ever helped for very long strings.

I thought we made study a noop some time back? If we bring it back
maybe they will optimize this.

Studying &PL_sv_yes and &PL_sv_no?

(Sorry for the cryptic response, but I don’t know how to explain it
better than I did above.)

I don't find it cryptic so no worries. And yes while it is silly to
study these values IMO it makes no sense to warn about a no-op. Others
might disagree with me tho.

Aha, now I see what I forgot to say.

chr /foo/ warns about ambiguity. So does every other operator with a ;$
prototype, *except* study. Why do we need a special exception for study?

Im inclined to say that we should just remove *any* study based code
as it has been a no-op long enough that if it made a difference we
would have heard.

I have already removed it. :-)

Don’t remove study itself, though. I use it all the time so I can set a
breakpoint on Perl_pp_study in a C debugger. :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2012

From @demerphq

On 16 October 2012 02​:40, Father Chrysostomos via RT
<perlbug-comment@​perl.org> wrote​:

On Mon Oct 15 07​:51​:23 2012, demerphq wrote​:

On 15 October 2012 16​:47, Father Chrysostomos via RT
<perlbug-comment@​perl.org> wrote​:

On Mon Oct 15 02​:54​:07 2012, demerphq wrote​:

On 14 October 2012 23​:52, Father Chrysostomos <perlbug-
followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #115264]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115264 >

commit c277df4
Author​: Ilya Zakharevich <ilya@​math.berkeley.edu>
Date​: Sat Nov 15 19​:29​:39 1997 -0500

Jumbo regexp patch applied \(with minor fix\-up tweaks\)&#8203;:
    Subject&#8203;: Version 7 of Jumbo RE patch available

p4raw\-id&#8203;: //depot/perl@&#8203;267

This hunk makes no sense​:

@​@​ -2389,7 +2389,11 @​@​ yylex(void)
case '/'​: /* may either be division or pattern
*/
case '?'​: /* may either be conditional or
pattern */
if (expect != XOPERATOR) {
- check_uni();
+ /* Disable warning on "study /blah/" */
+ if (oldoldbufptr == last_uni
+ && (*last_uni != 's' || s - last_uni < 5
+ || memNE(last_uni, "study", 5) ||
isALNUM(last_uni[5])))
+ check_uni();
s = scan_pat(s);
TERM(sublex_start());
}

Studying the boolean returned by /.../ is not helpful, as no match
is likely to be performed on that boolean. And if it were, study()
would not help, as it has only ever helped for very long strings.

I thought we made study a noop some time back? If we bring it back
maybe they will optimize this.

Studying &PL_sv_yes and &PL_sv_no?

(Sorry for the cryptic response, but I don’t know how to explain it
better than I did above.)

I don't find it cryptic so no worries. And yes while it is silly to
study these values IMO it makes no sense to warn about a no-op. Others
might disagree with me tho.

Aha, now I see what I forgot to say.

chr /foo/ warns about ambiguity. So does every other operator with a ;$
prototype, *except* study. Why do we need a special exception for study?

You seem to have a stronger opinion about warning than I do about not,
so go ahead. :-)

Im inclined to say that we should just remove *any* study based code
as it has been a no-op long enough that if it made a difference we
would have heard.

I have already removed it. :-)

Don’t remove study itself, though. I use it all the time so I can set a
breakpoint on Perl_pp_study in a C debugger. :-)

Heh, cool idea.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2012

From @iabyn

On Mon, Oct 15, 2012 at 05​:40​:12PM -0700, Father Chrysostomos via RT wrote​:

Don’t remove study itself, though. I use it all the time so I can set a
breakpoint on Perl_pp_study in a C debugger. :-)

Personally I always use pp_sin - it's less to type :-)

--
"There's something wrong with our bloody ships today, Chatfield."
  -- Admiral Beatty at the Battle of Jutland, 31st May 1916.

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2012

From @bulk88

On Tue Oct 16 02​:15​:51 2012, davem wrote​:

On Mon, Oct 15, 2012 at 05​:40​:12PM -0700, Father Chrysostomos via RT
wrote​:

Don’t remove study itself, though. I use it all the time so I can set a
breakpoint on Perl_pp_study in a C debugger. :-)

Personally I always use pp_sin - it's less to type :-)

I use a |system('pause');| to create C breakpoint, it is a call to the
Win32 shell (Google says "read -p "Press any key"" is equivelent). I've
been thinking that dump() should be repurposed for C breakpoints or
calls to some platform specific diagnostic tool (memory dumper???)
sometime soon. Does anyone use unexec on perl? comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants