Smalltalk › Cincom › VisualWorks

Glossary for messages given by VW's code-critic..

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

15 messages Options

Rick Flower

Glossary for messages given by VW's code-critic..

Does anyone have a list of what the various messages spat out by the VW
code critic mean?

For example, tonight I see a new message I've never seen before -- and
I've got no idea what it means :

"Method with full blocks"

Most of the messages spat out are guessable as to what they mean, but I
occasionally
get ones that leave me stumped.. Anyway, I just thought I'd ask!

Thanks!

Reinout Heeck-2

Re: Glossary for messages given by VW's code-critic..

Rick Flower wrote:
> Does anyone have a list of what the various messages spat out by the
> VW code critic mean?
>

Of course: Google does.

The first hit searching for "Method with full blocks" pointed to the
following:

http://newsfeeds.com/archive/comp-lang-smalltalk/msg02088.html

> For example, tonight I see a new message I've never seen before -- and
> I've got no idea what it means :
>
> "Method with full blocks"
>
> Most of the messages spat out are guessable as to what they mean, but
> I occasionally
> get ones that leave me stumped.. Anyway, I just thought I'd ask!
>

R
-

Mark Roberts

Re: Glossary for messages given by VW's code-critic..

In reply to this post by Rick Flower

At 03:17 PM 5/11/2007, Rick Flower wrote:

Does anyone have a list of what the various messages spat out by the VW code critic mean?

There is a complete and concise glossary in the VisualWorks documentation. Have a look at "Code Critic Rules" in the Tools Guide:

http://www.cincomsmalltalk.com/documentation/vw7.4.1/ToolGuide.pdf

Here's one:

Methods with full blocks

Checks for methods that contain full blocks or create a context with the thisContext keyword. These methods are a place where inefficiencies can creep in. For example, a common reason why a full block is created is because a block assigns a temporary variable that is not defined inside the block. If the temporary variable is only used inside the block, then the definition of the temporary should be moved inside the block. The "move to inner scope" refactoring can be used to correct this.

HTH,

M. Roberts
Cincom Systems, Inc.

Alejandro F. Reimondo

Re: Glossary for messages given by VW's code-critic..

Hi,

>If the temporary variable is only used inside the block,

> then the definition of the temporary should be

> moved inside the block.

This action requires the knowledge of the trick

and attention to coding correctly while thinking in

objects (so, it will not be written right most of the times).

I think this kind of trick "should be" detected and

realized by the compiler (if configured for optimization)

and the responsibility not be delegated to the person

who is thinking in how to solve a message for

an object at hand.

Saying that one must write code efficiently make us think

in terms of coding and not in objects (like +20years ago

when using programming languages to write

programs).

I also think that a product like VW has this

trick implemented and provably the confusion

is generated by the words written in the guide.

best,
Ale.

----- Original Message -----

From: [hidden email]

To: [hidden email] ; [hidden email]

Sent: Friday, May 11, 2007 4:56 AM

Subject: Re: Glossary for messages given by VW's code-critic..

At 03:17 PM 5/11/2007, Rick Flower wrote:

Does anyone have a list of what the various messages spat out by the VW code critic mean?

There is a complete and concise glossary in the VisualWorks documentation. Have a look at "Code Critic Rules" in the Tools Guide:

http://www.cincomsmalltalk.com/documentation/vw7.4.1/ToolGuide.pdf

Here's one:

Methods with full blocks

Checks for methods that contain full blocks or create a context with the thisContext keyword. These methods are a place where inefficiencies can creep in. For example, a common reason why a full block is created is because a block assigns a temporary variable that is not defined inside the block. If the temporary variable is only used inside the block, then the definition of the temporary should be moved inside the block. The "move to inner scope" refactoring can be used to correct this.

HTH,

M. Roberts
Cincom Systems, Inc.

Rick Flower

Re: Glossary for messages given by VW's code-critic..

In reply to this post by Reinout Heeck-2

Reinout Heeck wrote:

> Rick Flower wrote:
>> Does anyone have a list of what the various messages spat out by the
>> VW code critic mean?
>>
>
> Of course: Google does.
>
> The first hit searching for "Method with full blocks" pointed to the
> following:
>
> http://newsfeeds.com/archive/comp-lang-smalltalk/msg02088.html
>

Thanks.. I actually ran that one, but it didn't seem to answer what it
meant -- at least to me..

Rick Flower

Re: Glossary for messages given by VW's code-critic..

In reply to this post by Mark Roberts

Thanks Mark! I guess I missed that pdf when nosing around..

Mark Roberts wrote:

> At 03:17 PM 5/11/2007, Rick Flower wrote:
>> Does anyone have a list of what the various messages spat out by the
>> VW code critic mean?
>
> There is a complete and concise glossary in the VisualWorks
> documentation. Have a look at "Code Critic Rules" in the Tools Guide:
>
> http://www.cincomsmalltalk.com/documentation/vw7.4.1/ToolGuide.pdf
>
> Here's one:
>
> *Methods with full blocks
>
> *Checks for methods that contain full blocks or create a context with
> the thisContext keyword. These methods are a place where
> inefficiencies can creep in. For example, a common reason why a full
> block is created is because a block assigns a temporary variable that
> is not defined inside the block. If the temporary variable is only
> used inside the block, then the definition of the temporary should be
> moved inside the block. The "move to inner scope" refactoring can be
> used to correct this.
>
> HTH,
>
> M. Roberts
> Cincom Systems, Inc.

Rick Flower

Re: Glossary for messages given by VW's code-critic..

In reply to this post by Mark Roberts

Mark Roberts wrote:

>
> *Methods with full blocks
>
> *Checks for methods that contain full blocks or create a context with
> the thisContext keyword. These methods are a place where inefficiencies
> can creep in. For example, a common reason why a full block is created
> is because a block assigns a temporary variable that is not defined
> inside the block. If the temporary variable is only used inside the
> block, then the definition of the temporary should be moved inside the
> block. The "move to inner scope" refactoring can be used to correct this.

Ok.. After looking at the code (see below), I don't see what it's seeing
that causes this to be flagged -- am I missing something?

with: aBlock
| div |

canvas forgetCurrentBrush.
div := canvas div.
div id: (self id);
class: 'bd ', class;
style: 'overflow: visible; height: auto;', style;
with: [
canvas unorderedList class: 'first-of-type'; with: aBlock
]

Dennis smith-4

Re: Glossary for messages given by VW's code-critic..

Rick Flower wrote:

> Mark Roberts wrote:
>>
>> *Methods with full blocks
>>
>> *Checks for methods that contain full blocks or create a context with
>> the thisContext keyword. These methods are a place where
>> inefficiencies can creep in. For example, a common reason why a full
>> block is created is because a block assigns a temporary variable that
>> is not defined inside the block. If the temporary variable is only
>> used inside the block, then the definition of the temporary should be
>> moved inside the block. The "move to inner scope" refactoring can be
>> used to correct this.
>
> Ok.. After looking at the code (see below), I don't see what it's seeing
> that causes this to be flagged -- am I missing something?
>
> with: aBlock
> | div |
>
> canvas forgetCurrentBrush.
> div := canvas div.
> div id: (self id);
> class: 'bd ', class;
> style: 'overflow: visible; height: auto;', style;
> with: [
> canvas unorderedList class: 'first-of-type'; with: aBlock
> ]

The with: [...] block contains "aBlock" which is external to the with:
[] block.
Anything in a block that is defined outside the block will cause a full
block.

--
Dennis Smith +1 416.798.7948
Cherniak Software Development Corporation Fax: +1 416.798.0948
509-2001 Sheppard Avenue East [hidden email]
Toronto, ON M2J 4Z8 sip:[hidden email]
Canada http://www.CherniakSoftware.com
Entrance off Yorkland Blvd south of Sheppard Ave east of the DVP

Rick Flower

Re: Glossary for messages given by VW's code-critic..

Dennis Smith wrote:

> The with: [...] block contains "aBlock" which is external to the with:
> [] block.
> Anything in a block that is defined outside the block will cause a full
> block.

Doh! I guess I can't really make a "fix" for that.. I'll just ignore
that error/warning.. Thanks!

Eliot Miranda-2

Re: Glossary for messages given by VW's code-critic..

In reply to this post by Dennis smith-4

On 5/11/07, Dennis Smith <[hidden email]> wrote:

Rick Flower wrote:

> Mark Roberts wrote:
>>
>> *Methods with full blocks
>>
>> *Checks for methods that contain full blocks or create a context with
>> the thisContext keyword. These methods are a place where
>> inefficiencies can creep in. For example, a common reason why a full
>> block is created is because a block assigns a temporary variable that
>> is not defined inside the block. If the temporary variable is only
>> used inside the block, then the definition of the temporary should be
>> moved inside the block. The "move to inner scope" refactoring can be
>> used to correct this.
>
> Ok.. After looking at the code (see below), I don't see what it's seeing
> that causes this to be flagged -- am I missing something?
>
> with: aBlock
>   | div |
>
>   canvas forgetCurrentBrush.
>   div := canvas div.
>   div id: (self id);
>     class: 'bd ', class;
>     style: 'overflow: visible; height: auto;', style;
>     with: [
>       canvas unorderedList class: 'first-of-type'; with: aBlock
>     ]

The with: [...]  block contains "aBlock" which is external to the with:
[] block.
Anything in a block that is defined outside the block will cause a full
block.

Um, not so. The compiler should be able to copy the value of aBlock. Its an argument and shoudn't change. Have you, Rick, checked the bytecode of the method to see if its really creating a full block?

In any case I would ignore full block warnings. From 5i onwards the contxt/block implemetation is much improved and full blocks are rarer and cost much less than previously.

Rick Flower

Re: Glossary for messages given by VW's code-critic..

Eliot Miranda wrote:

> Um, not so. The compiler should be able to copy the value of aBlock.
> Its an argument and shoudn't change. Have you, Rick, checked the
> bytecode of the method to see if its really creating a full block?

No.. mostly because I've got no clue how to do that.. In any case, based
on your comment below I'm not too convinced I need to be worried about
it (perhaps until I profile it and find it slow).. Not that I'd know
a full-block from something else while staring at byte-code.. (8->

As a side-note, I refactored a little part of that same code and no
longer get this warning anymore (see updated code below for your amusement):

with: aBlock
| div |

canvas forgetCurrentBrush.
div := canvas div.
"Optionally put the id if one was given to us.."
(id isEmptyOrNil) ifFalse: [
div id: (self id).
].
div class: 'bd ', class;
style: 'overflow: visible; height: auto;', style;
with: [
canvas unorderedList class: 'first-of-type'; with: aBlock
]

Eliot Miranda-2

Re: Glossary for messages given by VW's code-critic..

On 5/11/07, Rick Flower <[hidden email]> wrote:

Eliot Miranda wrote:

> Um, not so. The compiler should be able to copy the value of aBlock.
> Its an argument and shoudn't change. Have you, Rick, checked the
> bytecode of the method to see if its really creating a full block?

No.. mostly because I've got no clue how to do that..

If you can easily get the code back ten is easy. Just inspect the method and select "bytecode" in the inspector. You can inspect the method by (I think) loading Programming Extensions and using "inspect" on the browser's selector list menu. Or you can do it longhand via e.g.

(MyClass compiledMethodAt: #with:) inspect

In any case, based
on your comment below I'm not too convinced I need to be worried about
it (perhaps until I profile it and find it slow).. Not that I'd know
a full-block from something else while staring at byte-code.. (8->

:) look for things like

8 <F7 00 01> make full copying block (1)

26 <CF 03> make full block

As a side-note, I refactored a little part of that same code and no
longer get this warning anymore (see updated code below for your amusement):

with: aBlock
   | div |

   canvas forgetCurrentBrush.
   div := canvas div.
   "Optionally put the id if one was given to us.."
   (id isEmptyOrNil) ifFalse: [
       div id: (self id).
   ].
   div class: 'bd ', class;
     style: 'overflow: visible; height: auto;', style;
     with: [
         canvas unorderedList class: 'first-of-type'; with: aBlock
     ]

Looks like the oracle has lost an eye :) I bet you the compiler generates the same block creation btecode in both cases.

Rick Flower

Re: Glossary for messages given by VW's code-critic..

Eliot Miranda wrote:
>
>
> On 5/11/07, *Rick Flower* <[hidden email]

> If you can easily get the code back ten is easy. Just inspect the
> method and select "bytecode" in the inspector. You can inspect the
> method by (I think) loading Programming Extensions and using "inspect"
> on the browser's selector list menu. Or you can do it longhand via e.g.
>
> (MyClass compiledMethodAt: #with:) inspect
>
Thanks Eliot.. I'll check it out..

Jarek@GMX

Re: Glossary for messages given by VW's code-critic..

In reply to this post by Alejandro F. Reimondo

Alejandro F. Reimondo wrote:

> Hi,
>
> >If the temporary variable is only used inside the block,
> > then the definition of the temporary should be
> > moved inside the block.
>
> This action requires the knowledge of the trick
> and attention to coding correctly while thinking in
> objects (so, it will not be written right most of the times).
> I think this kind of trick "should be" detected and
> realized by the compiler (if configured for optimization)

I often find myself putting the temps outside of the block - it
simplifies debugging greatly as you can review what the temp was even
when you got out of the block. I found it helps to get ones head around
what was going on in some of the methods...

Regards, Jaroslaw.

jWarrior

Re: Glossary for messages given by VW's code-critic..

Jaroslaw Podgajny wrote:

> Alejandro F. Reimondo wrote:
>
>> Hi,
>>
>> >If the temporary variable is only used inside the block,
>> > then the definition of the temporary should be
>> > moved inside the block.
>>
>> This action requires the knowledge of the trick
>> and attention to coding correctly while thinking in
>> objects (so, it will not be written right most of the times).
>> I think this kind of trick "should be" detected and
>> realized by the compiler (if configured for optimization)
>
> I often find myself putting the temps outside of the block - it
> simplifies debugging greatly as you can review what the temp was even
> when you got out of the block. I found it helps to get ones head
> around what was going on in some of the methods...

As Master said to Grasshopper, make it work, make it right, make it fast
(if necessary). ;-)

Premature optimization is the root of all evil, or at least most
unnecessary. Humans are very bad at predicting performance, which is why
it is good to ask Mr. Computer what he thinks is going on, but only
after the code works. Then refactor as necessary.

We have people on our project (J__) who fret about the initial size of
OrderedCollections, while ignoring more fundamental issues such as what
does it mean when an object stands up or dies -- IOW, what is known
after an object achieves some state.

Drives me nuts. Anybody focused at this low level oughta be coding in
assembler.

Smalltalk still rules,

Donald

>
>
>
> Regards, Jaroslaw.
>