Operator precedence (or, I'm an idiot)

[Mur]

I'm embarassed to admit how long I've been writing Perl, and had never caught on to this little pitfall:

  my $a = 1 and 0;

Since the and operator has lower precedence than =, this sets $a to "1". Apparently no warning is issued, either.

In my defense, the actual code in which this came to light was a bit more complex:

    my $can_remote =
      defined($cfg->remote) and
      (($cfg->get('remote') eq 'all') or
      grep($type eq $_, split(/\s+|,\s*/,$cfg->get('remote'))));

Of course, changing and to && solves the problem. However, now I'm faced with a big problem: How do I scan 100,000 lines of Perl in our application code base to look for other places where I've done this?!?

--
Jeff Boes Database Engineer Nexcerpt, Inc.	vox 269.226.9550 ext 24 fax [phone]  http://www.nexcerpt.com
...Nexcerpt...Connecting People With Expertise

[hardburn]

Buy a lot of Jolt and be prepared to work long hours. Your other alternative is to write a Perl parser to check for all occurances of 'and' and 'or' in an expression.

You could speed up the process using grep (the *nix command, not the builtin) to search for all occurances of 'and' and 'or' in your code and print the line numbers.

----
Reinvent a rounder wheel.

Note: All code is untested, unless otherwise stated

[bronto]

If you are on a UNIX system, there is a full non-perl solution (untested!); this should work on a bourne-like shell:

for FILE in `find /dir -type f -name \*.pl -print` ; do sed -e 's/and/&&/g' $FILE > $FILE.new ; done

After that you should go through all .new files and see if you actually needed the change. I am not sure that you really want to change every and in &&

If you are sure you don't need to double-check the changed files, you could then substitute that sed with an analogous one-liner using perl -pi.bak 's/and/&&/g' (untested again) and keep the .bak files in a safe place if, one day, you'll find a wrong change.

Update: Oooooopssss! hardburn is right!!! You need to do the changes iteratively or write something to reparse your code... sorry, I am an idiot, too!

Ciao!
--bronto

---

The very nature of Perl to be like natural language--inconsistant and full of dwim and special cases--makes it impossible to know it all without simply memorizing the documentation (which is not complete or totally correct anyway).
--John M. Dlugosz

[belden]

You've solved a non-trivial problem in a trivial fashion. That's bound to cause problems. Running a blind sed on your perl code is inadvisable, to say the least. There's no way for either of your examples to *not* make an inappropriate change in comments, pod, or any string that happens to contain 'and':


   sub sandbox {
     my ($this, $that) = @_;
     blarf($this) and mungle($that);
     # etc.
   }

Both of your examples will re-write that as:

   sub s&&box {
     my ($this, $that) = @_;
     blarf($this) && mungle($that);
     # etc.
   }

Plus, there might be other programmers working on the project that don't have this and and && confusion. As you've already realized, there's no quick fix for the OP. Either examine each file, or put a bit more time into writing a smarter parser.

I did ++ you for having one saving grace: re-writing the code to temp files rather than over the original code, and keeping the original stuff around.

I know that this is stuff you already realized - but there's a few other replies similar to yours, and I happened to choose yours to reply to :)

blyman
setenv EXINIT 'set noai ts=2'

[rir]

use warnings;

Update: This will catch only some cases.

[tye]

Just another reason that grep (or map) in a void context should generate a warning.

I'm also curious why the simple version of your code reports

    Found = in conditional, should be ==

but the full version does not.
                - tye

[rir]

With the routine the $var may vary.

Even with:

  sub rout {
       &blah;
        1;
  }

there may be side effects.

[tye]

Of course there may be side effects. That is why you don't have to use -w. But there is no good reason to use grep and/or map solely for their side effects and there are good reasons to not do so. The warning would be:

    Use of map/grep in void context, use foreach instead

If you have some perverse reason for doing such, then just turn off that warning. For people wanting help in catching simple errors, the warning will be useful.
                - tye

[allolex]

Since I do "stupid" stuff like this all the time (and not just while coding), I like to use my lovely and user-friendly friend Kate (in KDE, of course) to regex search all lines for whatever and then to ask me if I want to replace whatever comes up. darkphorm suggested VIM, I'm suggesting Kate. (Frankly, I'm not sure you'd need regex support, but Kate even has a nifty regular expression editor if you need it.)

--
Allolex

[Hero Zzyzzx]

command-line grep is pretty useful:
grep -R " and " /path/to/code/base/*.pm | less
will give you a good start.

-Any sufficiently advanced technology is
indistinguishable from doubletalk.

[jonadab]

Yep. The spelled-out logical operators (and and or anyway; not sure about not) have what the Camel describes as "ultra-low precedence". That's exactly what makes them so useful: almost no matter what the context is, you can be pretty much sure that they'll short-circut unless the whole left hand side evaluates to true (if you use and) or false (if you use or). Hence the idiom or die "Aaaack! $!, $!, the sky is falling, $!\n"; the die won't trigger unless the whole left side fails. If you use || for that you have to throw parens around everything.

---

for(unpack("C*",'GGGG?GGGG?O__\?WccW?{GCw?Wcc{?Wcc~?Wcc{?~cc'
.'W?')){$j=$_-63;++$a;for$p(0..7){$h[$p][$a]=$j%2;$j/=2}}for$
p(0..7){for$a(1..45){$_=($h[$p-1][$a])?'#':' ';print}print$/}

[darkphorm]

How many actual files are there in the system? You could just run a regular expression replace in VIM. I probably don't have the expression right but something similar might work

s/(\$\w)\ \=\ (\d+)\ and\ (\d+)/$1\ \=\ ($2)\ \&\&\ ($3)/gi
You might also be able to this with a perl script, or perhaps special tricks with a commandline grep.