From 47c9b5205a0da19efd133bd1de839e5920b5223d Mon Sep 17 00:00:00 2001
From: Alena Laskavaia <elaskavaia.cdt@gmail.com>
Date: Fri, 26 Mar 2010 15:02:42 +0000
Subject: [PATCH] [281187]  checker was doing the opposite - implemented
 properly

---
 .../org.eclipse.cdt.codan.checkers/plugin.xml |  4 +-
 .../internal/checkers/CatchUsesReference.java | 46 ++++++++++++++-----
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml
index d00ee1355ef..5f03f047414 100644
--- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml
+++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml
@@ -50,8 +50,10 @@
       <problem
             category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
             defaultSeverity="Warning"
+            description="Catching by reference is recommended by C++ experts, &quot;Throw by value, catch by reference&quot;. &#x0A;  For one thing, this avoids copying and potentially slicing the exception."
             id="org.eclipse.cdt.codan.internal.checkers.CatchUsesReference"
-            name="Catch uses reference to exception">
+            name="Catching by reference is recommended"
+            messagePattern="Catching by reference is recommended for non-basic types">
       </problem>
       </checker>
       <checker
diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CatchUsesReference.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CatchUsesReference.java
index d484f9204e1..d3903c32b9c 100644
--- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CatchUsesReference.java
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CatchUsesReference.java
@@ -7,24 +7,31 @@
  *
  * Contributors:
  *    Alena Laskavaia  - initial API and implementation
- *******************************************************************************/ 
+ *******************************************************************************/
 
 package org.eclipse.cdt.codan.internal.checkers;
 
 import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
 import org.eclipse.cdt.core.dom.ast.IASTPointerOperator;
 import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTStatement;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
+import org.eclipse.cdt.core.dom.ast.IBasicType;
+import org.eclipse.cdt.core.dom.ast.IBinding;
+import org.eclipse.cdt.core.dom.ast.IType;
+import org.eclipse.cdt.core.dom.ast.ITypedef;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCatchHandler;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamedTypeSpecifier;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTReferenceOperator;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTryBlockStatement;
 
 /**
- * @author Alena
+ * Catching by reference is recommended by C++ experts, for example Herb Sutter/Andrei Alexandresscu "C++ Coding Standards", Rule 73 "Throw by value, catch by reference". 
+ * For one thing, this avoids copying and potentially slicing the exception.
  *
  */
 public class CatchUsesReference extends AbstractIndexAstChecker {
@@ -34,26 +41,44 @@ public class CatchUsesReference extends AbstractIndexAstChecker {
 		// traverse the ast using the visitor pattern.
 		ast.accept(new OnCatch());
 	}
-	
+
 	class OnCatch extends ASTVisitor {
 		OnCatch() {
 			shouldVisitStatements = true;
 		}
+
 		public int visit(IASTStatement stmt) {
 			if (stmt instanceof ICPPASTTryBlockStatement) {
 				ICPPASTTryBlockStatement tblock = (ICPPASTTryBlockStatement) stmt;
 				ICPPASTCatchHandler[] catchHandlers = tblock.getCatchHandlers();
-				for (int i = 0; i < catchHandlers.length; i++) {
+				next: for (int i = 0; i < catchHandlers.length; i++) {
 					ICPPASTCatchHandler catchHandler = catchHandlers[i];
-					if (usesReference(catchHandler)) {
-						reportProblem(ER_ID, catchHandler.getDeclaration(), "Catch clause uses reference in declaration of exception");
+					IASTDeclaration decl = catchHandler.getDeclaration();
+					if (decl instanceof IASTSimpleDeclaration) {
+						IASTSimpleDeclaration sdecl = (IASTSimpleDeclaration) decl;
+						IASTDeclSpecifier spec = sdecl.getDeclSpecifier();
+						if (!usesReference(catchHandler)) {
+							if (spec instanceof ICPPASTNamedTypeSpecifier) {
+								IBinding typeName = ((ICPPASTNamedTypeSpecifier) spec).getName().getBinding();
+								// unwind typedef chain
+								while (typeName instanceof ITypedef) {
+									IType t = ((ITypedef) typeName).getType();
+									if (t instanceof IBasicType) continue next;
+									if (t instanceof IBinding) typeName = (IBinding) t;
+									else break;
+								}
+
+								reportProblem(ER_ID, decl);
+							}
+						}
 					}
 				}
-		
+
 				return PROCESS_SKIP;
 			}
 			return PROCESS_CONTINUE;
 		}
+
 		/**
 		 * @param catchHandler
 		 * @return
@@ -67,16 +92,13 @@ public class CatchUsesReference extends AbstractIndexAstChecker {
 					IASTPointerOperator[] pointerOperators = d.getPointerOperators();
 					for (int j = 0; j < pointerOperators.length; j++) {
 						IASTPointerOperator po = pointerOperators[j];
-						if (po instanceof ICPPASTReferenceOperator) {
-							return true;
-						}
-						
+						if (po instanceof ICPPASTReferenceOperator) { return true; }
+
 					}
 				}
 			}
 			return false;
 		}
 	}
-	
 
 }