Bug 535274: Allow attributes on namespace definitions

Implementation and tests.

Change-Id: I0cca9dea8630ae66b005856338342b4173c48216
Signed-off-by: Hansruedi Patzen <hansruedi.patzen@hsr.ch>
Signed-off-by: Thomas Corbat <tcorbat@hsr.ch>

core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPAttributeTests.java

@@ -44,6 +44,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTEnumerationSpecifier;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDefinition;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTIfStatement;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamespaceDefinition;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTParameterDeclaration;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTReferenceOperator;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTSimpleDeclSpecifier;
@@ -537,4 +538,22 @@ public class AST2CPPAttributeTests extends AST2TestBase {
 		IASTTranslationUnit tu = parseAndCheckBindings(getAboveComment(), ParserLanguage.CPP, true);
 		checkAttributeRelations(getAttributeSpecifiers(tu), ICPPASTParameterDeclaration.class, ICPPASTSimpleDeclSpecifier.class);
 	}
+
+	//namespace [[attr]] NS {}
+	public void testAttributedNamedNamespace_Bug535274() throws Exception {
+		IASTTranslationUnit tu = parseAndCheckBindings(getAboveComment(), ParserLanguage.CPP, true);
+		checkAttributeRelations(getAttributeSpecifiers(tu), ICPPASTNamespaceDefinition.class);
+	}
+
+	//namespace [[attr]] {}
+	public void testAttributedUnnamedNamespace_Bug535274() throws Exception {
+		IASTTranslationUnit tu = parseAndCheckBindings(getAboveComment(), ParserLanguage.CPP, true);
+		checkAttributeRelations(getAttributeSpecifiers(tu), ICPPASTNamespaceDefinition.class);
+	}
+
+	//namespace NS __attribute__((__visibility__("default"))) {}
+	public void testGnuAndCppMixedAttributedNamedNamespace_Bug535274() throws Exception {
+		IASTTranslationUnit tu = parseAndCheckBindings(getAboveComment(), ParserLanguage.CPP, true);
+		checkAttributeRelations(getAttributeSpecifiers(tu), ICPPASTNamespaceDefinition.class);
+	}
 }

core/org.eclipse.cdt.core.tests/resources/rewrite/ASTWriterDeclarationTestSource.awts

@@ -178,3 +178,19 @@ inline namespace InlnNS
 void f([[attr1]] int p1, int [[attr2]] p2, [[attr3]] int p3)
 {
 }
+
+//!Attributed anonymous namespace
+//%CPP
+namespace [[foo]] 
+{
+
+
+}
+
+//!Mixed CPP and GNU attributes on named namespace
+//%CPP GNU
+namespace [[foo]] FOO __attribute__((__visibility__("default")))
+{
+
+
+}

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java

@@ -2439,8 +2439,8 @@ public abstract class AbstractGNUSourceCodeParser implements ISourceCodeParser {
     		return null;
 
     	IASTAttributeList result = nodeFactory.newGCCAttributeList();
-		final int startOffset = consume().getOffset();
+    	final int startOffset = LA().getOffset();
-    	int endOffset = startOffset;
+    	int endOffset = consume().getEndOffset();
     	if (LT(1) == IToken.tLPAREN) {
     		consume();
     		consume(IToken.tLPAREN);
@@ -2464,8 +2464,7 @@ public abstract class AbstractGNUSourceCodeParser implements ISourceCodeParser {
     		consumeOrEOC(IToken.tRPAREN);
     		endOffset = consumeOrEOC(IToken.tRPAREN).getEndOffset();
     	}
-    	setRange(result, startOffset, endOffset);
+    	return setRange(result, startOffset, endOffset);
-    	return result;
     }
 
 	protected IASTAttribute singleAttribute() throws EndOfFileException, BacktrackException {

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTNamespaceDefinition.java

@@ -125,6 +125,9 @@ public class CPPASTNamespaceDefinition extends CPPASTAttributeOwner implements I
 	        }
 		}
 
+		if (!acceptByCPPAttributeSpecifiers(action))
+			return false;
+
 		if (fName != null && !fName.accept(action))
 			return false;
 

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java

@@ -2676,6 +2676,8 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser {
 		}
 		consume(IToken.t_namespace);
 
+		List<IASTAttributeSpecifier> attributeSpecifiers = attributeSpecifierSeq();
+
 		// optional name
 		ICPPASTName name = null;
 		if (LT(1) == IToken.tIDENTIFIER) {
@@ -2686,7 +2688,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser {
 		}
 
 		// bug 195701, gcc 4.2 allows visibility attribute for namespaces.
-		List<IASTAttributeSpecifier> attributeSpecifiers = __attribute_decl_seq(true, false);
+		attributeSpecifiers = CollectionUtils.merge(attributeSpecifiers, __attribute_decl_seq(true, false));
 
 		if (LT(1) == IToken.tLBRACE) {
 			ICPPASTNamespaceDefinition outer = null;

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVisitor.java

@@ -3088,4 +3088,15 @@ public class CPPVisitor extends ASTQueries {
 		ArrayUtil.reverse(operands);
 		return operands;
 	}
+
+	/**
+	 * Determines whether the given {@code namespace} definition denotes
+	 * an anonymous namespace.
+	 * @param namespace
+	 * @return {@code true} if the {@code namespace} is anonymous, false otherwise
+	 */
+	public static boolean isAnonymousNamespace(ICPPASTNamespaceDefinition namespace) {
+		IASTName name = namespace.getName();
+		return name == null || name.toString().isEmpty();
+	}
 }

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/DeclarationWriter.java

@@ -207,6 +207,7 @@ public class DeclarationWriter extends NodeWriter {
 			scribe.printStringSpace(Keywords.INLINE);
 		}
 		scribe.printStringSpace(Keywords.NAMESPACE);
+		writeCPPAttributes(namespaceDefinition, EnumSet.of(SpaceLocation.AFTER));
 		namespaceDefinition.getName().accept(visitor);
 		writeGCCAttributes(namespaceDefinition, EnumSet.of(SpaceLocation.BEFORE));
 		if (!hasTrailingComments(namespaceDefinition.getName())) {

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/CodeFormatterVisitor.java

@@ -18,6 +18,7 @@ import java.util.Collections;
 import java.util.EmptyStackException;
 import java.util.List;
 import java.util.Stack;
+import java.util.function.Predicate;
 
 import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
@@ -104,6 +105,7 @@ import org.eclipse.cdt.core.dom.ast.c.ICASTCompositeTypeSpecifier;
 import org.eclipse.cdt.core.dom.ast.c.ICASTDesignatedInitializer;
 import org.eclipse.cdt.core.dom.ast.c.ICASTDesignator;
 import org.eclipse.cdt.core.dom.ast.c.ICASTVisitor;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTAttributeList;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTBinaryExpression;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCastExpression;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCatchHandler;
@@ -149,6 +151,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTUsingDirective;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTVisibilityLabel;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTVisitor;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTWhileStatement;
+import org.eclipse.cdt.core.dom.ast.gnu.IGCCASTAttributeList;
 import org.eclipse.cdt.core.dom.ast.gnu.c.ICASTKnRFunctionDeclarator;
 import org.eclipse.cdt.core.formatter.DefaultCodeFormatterConstants;
 import org.eclipse.cdt.core.formatter.DefaultCodeFormatterOptions;
@@ -1103,7 +1106,13 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 		// namespace <name>
 		scribe.printNextToken(Token.t_namespace, false);
 		scribe.space();
-		node.getName().accept(this);
+		formatLeadingAttributes(node, ICPPASTAttributeList.class::isInstance);
+		boolean isNamedNamespace = !CPPVisitor.isAnonymousNamespace(node);
+		if (isNamedNamespace) {
+			IASTName name = node.getName();
+			name.accept(this);
+		}
+		formatAttributes(node, isNamedNamespace, false, IGCCASTAttributeList.class::isInstance);
 
 		// member declarations
 		IASTDeclaration[] memberDecls= node.getDeclarations();
@@ -1537,13 +1546,22 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 		return -1;
 	}
 
+	private void formatLeadingAttributes(IASTAttributeOwner owner) {
+		formatAttributes(owner, false, true);
+	}
+
 	/**
 	 * Formats the attributes leading a node.
 	 * Same as {@code formatAttributes(owner, false, true);}
 	 * @param owner Node containing attributes
+	 * @param filter Filter predicate for specifying which attributes to print
 	 */
-	private void formatLeadingAttributes(IASTAttributeOwner owner) {
+	private void formatLeadingAttributes(IASTAttributeOwner owner, Predicate<IASTAttributeSpecifier> predicate) {
-		formatAttributes(owner, false, true);
+		formatAttributes(owner, false, true, predicate);
+	}
+
+	private void formatAttributes(IASTAttributeOwner owner, boolean printLeadingSpace, boolean printTrailingSpace) {
+		formatAttributes(owner, printLeadingSpace, printTrailingSpace, unsused -> true);
 	}
 
 	/**
@@ -1552,8 +1570,10 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 	 * @param owner Node containing attributes
 	 * @param printLeadingSpace Print a space before the first attribute
 	 * @param printTrailingSpace Print a space after the last attribute
+	 * @param filter Filter predicate for specifying which attributes to print
 	 */
-	private void formatAttributes(IASTAttributeOwner owner, boolean printLeadingSpace, boolean printTrailingSpace) {
+	private void formatAttributes(IASTAttributeOwner owner, boolean printLeadingSpace,
+			boolean printTrailingSpace, Predicate<IASTAttributeSpecifier> filter) {
 		if (owner == null) {
 			return;
 		}
@@ -1563,7 +1583,9 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 				scribe.space();
 			}
 			for (IASTAttributeSpecifier attributeSpecifier : attributeSpecifiers) {
-				formatRaw(attributeSpecifier);
+				if (filter.test(attributeSpecifier)) {
+					formatRaw(attributeSpecifier);
+				}
 			}
 			if (printTrailingSpace) {
 				scribe.space();

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/CodeFormatterTest.java

@@ -15,8 +15,6 @@ package org.eclipse.cdt.ui.tests.text;
 import java.util.HashMap;
 import java.util.Map;
 
-import junit.framework.TestSuite;
-
 import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.jface.text.Document;
 import org.eclipse.jface.text.IDocument;
@@ -33,6 +31,8 @@ import org.eclipse.cdt.ui.tests.BaseUITestCase;
 import org.eclipse.cdt.internal.corext.util.CodeFormatterUtil;
 import org.eclipse.cdt.internal.formatter.align.Alignment;
 
+import junit.framework.TestSuite;
+
 /**
  * Tests for the CodeFormatter.
  *
@@ -3525,4 +3525,17 @@ public class CodeFormatterTest extends BaseUITestCase {
 	public void testAttributedGotoLabel_Bug535278_5() throws Exception {
 		assertFormatterResult();
 	}
+
+	//namespace[[foo]]{
+	//}
+	//namespace[[foo]]NS  __attribute__((__visibility__("default"))){
+	//}
+
+	//namespace [[foo]] {
+	//}
+	//namespace [[foo]] NS __attribute__((__visibility__("default"))) {
+	//}
+	public void testAttributedNamesapces_Bug535274() throws Exception {
+		assertFormatterResult();
+	}
 }