Bug 68603 - anomalous treatment of null when binding values
Summary: anomalous treatment of null when binding values
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.2   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 1.2.1   Edit
Assignee: Adrian Colyer CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 108894 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-06-25 09:33 EDT by Ganesh Sittampalam CLA
Modified: 2007-02-08 14:12 EST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ganesh Sittampalam CLA 2004-06-25 09:33:06 EDT
In the program below, the pointcut for foo(..) matches but the one for bar(..)
doesn't, despite the value passed and the type checked being the same in each
case. It seems that when ajc inserts a dynamic type check, it uses instanceof
which doesn't match null.


public class Test {
   static void foo(Test t) {
   }

   static void bar(Object t) {
   }

   public static void main(String[] args) {
      foo(null);
      bar(null);
   }
}

aspect Aspect {
   before(): execution(void foo(..)) && args(Test) {
      System.out.println("applied to foo");
   }
   before(): execution(void bar(..)) && args(Test) {
      System.out.println("applied to bar");
   }

}
Comment 1 Adrian Colyer CLA 2004-08-09 15:31:49 EDT
marked as target 1.2.1
Comment 2 Adrian Colyer CLA 2004-08-11 16:46:51 EDT
***Erik, please jump in if you disagree with this***

This is working as designed.

The static signature of bar only guarantees an Object argument, so 'null' can 
only be assumed to be an empty Object reference and there are no grounds for 
assuming that 'null' is a Test reference. Where would it end if we did that? A 
method taking an Object parameter would be matched by every args(RefType) 
pointcut expression you could ever construct when passed 'null'....
Comment 3 Ganesh Sittampalam CLA 2004-09-14 06:18:32 EDT
Reopening bug after discussing this with Adrian in person.

Having the assumed type of null depend on the declared type of the place it is
being passed to is extremely confusing, and potentially rather fragile.
args(...) etc is expected to be a *dynamic* test; the correct way to get a
static test is to specify the signature of the method.

The behaviour should be consistent one way or another. Since otherwise there
would be no way to match null at all, the best solution would be to allow null
to always match. 
Comment 4 Erik Hilsdale CLA 2004-09-29 18:59:12 EDT
I am recommending that this bug be re-closed as RESOLVED/NOTABUG.

Why?  Well, of course it is not an implementation bug.  The
implementation is entirely consistent with the specification.

Nor is it a specification bug, if "bug" here is read to mean
"oversight".  The specification is pretty clear that the
treatment of null is an exception to a more general rule, and
writing in that exception took effort.  It's not as if we forgot
to handle null, rather, we specifically handled it in a specific
way.

So this report is really about a perceived mis-specification of
the behavior.  So let's address that.  The three issues brought
up are that this behavior is

   * confusing,
   * fragile, and
   * inconsistent

I do not believe this to be true. 

* It's not extremely confusing to the target audience of the
  tool.  If it were, we would have many reports from actual users
  complaining about their confusion.  Confusion is a readily
  testable empirical property -- it leads to high-volume email
  lists.  Or, I suppose, it leads to non-adoption if the
  confusion is too great.  I don't believe this issue has caused
  our users to be so confused that they refuse to pick up the
  tool.

* While it may be "potentially rather fragile", the potential for
  fragility is in much PL design.  One of the points of doing
  AspectJ as a "popular language" rather than as a "calculus" is
  that we were less interested in potential fragility than in
  actual fragility.

* The behavior is exactly consistent: consistent with the
  well-specified semantics, which specifies an exception to a
  general rule.  Java's treatment of the null value and the null
  type is a similarly "consistent" exception. 

The reason we chose the particular exception we did was twofold:

* A cleaner (i.e., simpler, without an exception) rule that null
  either always or never matches would require us to insert many
  more dynamic tests than we currently do.  One of the features
  of AspectJ that enhanced adoption was that we would use the
  static information from the program to avoid inserting dynamic
  tests.  We found that in the cases where people weren't
  thinking about null, they were surprised by the (perceived)
  performance hit of an inserted dynamic test just to handle
  it... and when they were thinking about null, they weren't
  surprised that there is a special case for it.

* A cleaner rule may also have surprising (to programmers)
  results in pointcut matching, especially when interfaces come
  into play.  For example, if null always matches then args(List)
  might pick out execution join points for void
  setAccountId(Integer).

In short, language design is a different activity than language
calculus design, though the former is certainly informed by the
latter.  (As a Schemer this sometimes makes me sad.)  Sometimes
cleanliness is traded off for engineering properties like
utility, ease-of-adoption, and non-surprisingness.  That
trade-off should be made with taste, and it should be made
explicitly.  I believe AspectJ has done so in this case (and I
hope it has done so in most other cases).
Comment 5 Ganesh Sittampalam CLA 2004-09-30 08:38:04 EDT
We looked through the Programming Guide and Semantics Appendix carefully, but
didn't find any specification of this behaviour. Did we miss something, or is
there another document we should have read?
Comment 6 Adrian Colyer CLA 2005-09-09 06:48:11 EDT
*** Bug 108894 has been marked as a duplicate of this bug. ***
Comment 7 Adrian Colyer CLA 2005-09-09 07:12:26 EDT
The description of the args pointcut in the semantics appendix of the
programming guide has been updated to read:

 <varlistentry>
        <term><literal>args(<replaceable>Type</replaceable> or
<replaceable>Id</replaceable>, ...)</literal></term>
        <listitem>
          Picks out each join point where the arguments are instances of
          the appropriate type (or type of the identifier if using that form). A
          <literal>null</literal> argument is matched iff the static type of the 
          argument (declared parameter type or field type) is the same as, or a
subtype of,
          the specified args type. 
        </listitem>
      </varlistentry>
Comment 8 Eric Bodden CLA 2007-02-08 14:12:24 EST
Just adding to the discussion, since the issue came up again on the mailing list...

Technically, in Java "null" is always of type NullType, which is a subtype of any type. Hence, null can be assigned to any variable of any other reference type. Consequently, in order to be *consistent with Java* (which I think, AspectJ should be), I would consider the current behavior a flaw in the specification.

If people want to match on the declared type they can do so in the method pattern. If they use args one has to assume they mean the runtime type which is, as I said NullType.