c++/9750: Strange code in handling of COND? expressions in cp/call.c

[john dot carter at tait dot co dot nz]

>Number:         9750
>Category:       c++
>Synopsis:       Strange code in handling of COND? expressions in cp/call.c
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    unassigned
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Feb 19 02:26:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     john.carter@tait.co.nz
>Release:        gcc-3.2.2
>Organization:
>Environment:
All.
>Description:
This code is in gcc-3.2.2/gcc/cp/call.c line 1961

      /* Otherwise, the types should be pointers.  */
      if (!(TREE_CODE (type1) == POINTER_TYPE
	    || TYPE_PTRMEM_P (type1)
	    || TYPE_PTRMEMFUNC_P (type1))
	  || !(TREE_CODE (type2) == POINTER_TYPE
	       || TYPE_PTRMEM_P (type2)
	       || TYPE_PTRMEMFUNC_P (type2)))
	return candidates;
      
      /* We don't check that the two types are the same; the logic
	 below will actually create two candidates; one in which both
	 parameter types are TYPE1, and one in which both parameter
	 types are TYPE2.  */
	break;

      /* These arguments do not make for a legal overloaded operator.  */
      return candidates;

Note that the indentation for the "break" statement suggests that someone expects it was part of an "if" statement perhaps. And the "return candidates" code is now unreachable.

Probably not a bug, but a bit confuzzling.
>How-To-Repeat:
antic distributed as part of the jlint package found this.
>Fix:
Guess. Correct the indentation on the break and remove the "return candidates". This will make no difference to the code, but someone that grok's this code should have a look before this is done.

There are many instances of code like this...
  case BLOO :
    if (blah)
      break;
  case FLOO :
    if (flah)
      break;

Perhaps the author meant BLOO handling to fall through to FLOO handling. Perhaps he didn't. It would be nice to have a comment /* Fall through */ when it is known that this is meant to happen.
>Release-Note:
>Audit-Trail:
>Unformatted: