Fixed error because of new MontiMath version.

parent c638f7b0
......@@ -37,11 +37,11 @@
<properties>
<!-- .. SE-Libraries .................................................. -->
<monticore.version>4.5.3-SNAPSHOT</monticore.version>
<monticore.version>4.5.4-SNAPSHOT</monticore.version>
<se-commons.version>1.7.7</se-commons.version>
<mc.grammars.assembly.version>0.0.6-SNAPSHOT</mc.grammars.assembly.version>
<SIUnit.version>0.0.6-SNAPSHOT</SIUnit.version>
<Common-MontiCar.version>0.0.3-SNAPSHOT</Common-MontiCar.version>
<mc.grammars.assembly.version>0.0.6</mc.grammars.assembly.version>
<SIUnit.version>0.0.10-SNAPSHOT</SIUnit.version>
<Common-MontiCar.version>0.0.10-SNAPSHOT</Common-MontiCar.version>
<Math.version>0.0.10-SNAPSHOT</Math.version>
<!-- .. Libraries .................................................. -->
<guava.version>18.0</guava.version>
......@@ -69,6 +69,13 @@
</properties>
<dependencies>
<dependency>
<groupId>org.antlr</groupId>
<artifactId>antlr4-runtime</artifactId>
<version>4.7.1</version>
</dependency>
<dependency>
<groupId>de.se_rwth.commons</groupId>
<artifactId>se-commons-logging</artifactId>
......@@ -192,9 +199,7 @@
<artifactId>maven-deploy-plugin</artifactId>
<version>2.8.1</version>
<configuration>
<altDeploymentRepository>
internal.repo::default::file://${project.build.directory}/external-dependencies
</altDeploymentRepository>
<altDeploymentRepository>internal.repo::default::file://${project.build.directory}/external-dependencies</altDeploymentRepository>
</configuration>
</plugin>
<plugin>
......@@ -203,15 +208,11 @@
<version>0.12</version>
<configuration>
<server>github</server>
<message>Maven artifacts for ${project.version} ${project.artifactId}
</message> <!-- git commit message -->
<message>Maven artifacts for ${project.version} ${project.artifactId}</message> <!-- git commit message -->
<noJekyll>true</noJekyll> <!-- disable webpage processing -->
<outputDirectory>${project.build.directory}/external-dependencies
</outputDirectory> <!-- matches distribution management repository url above -->
<outputDirectory>${project.build.directory}/external-dependencies</outputDirectory> <!-- matches distribution management repository url above -->
<branch>refs/heads/master</branch> <!-- remote branch name -->
<includes>
<include>**/*</include>
</includes>
<includes><include>**/*</include></includes>
<repositoryName>external-dependencies</repositoryName> <!-- github repo name -->
<repositoryOwner>EmbeddedMontiArc</repositoryOwner> <!-- github username -->
<merge>true</merge>
......@@ -226,11 +227,29 @@
</execution>
</executions>
</plugin>
<!-- MontiCore Generation -->
<plugin>
<groupId>de.monticore.mojo</groupId>
<artifactId>monticore-maven-plugin</artifactId>
<version>${monticore.plugin}</version>
<configuration>
<skip>false</skip>
<script>de/monticore/monticore_noemf.groovy</script>
</configuration>
<dependencies>
<dependency>
<groupId>org.antlr</groupId>
<artifactId>antlr4</artifactId>
<version>4.7.1</version>
</dependency>
<!-- https://mvnrepository.com/artifact/org.antlr/antlr4-runtime -->
<dependency>
<groupId>org.antlr</groupId>
<artifactId>antlr4-runtime</artifactId>
<version>4.7.1</version>
</dependency>
</dependencies>
<executions>
<execution>
<goals>
......@@ -369,4 +388,4 @@
<url>file://${project.build.directory}/external-dependencies</url>
</snapshotRepository>
</distributionManagement>
</project>
</project>
\ No newline at end of file
......@@ -20,29 +20,31 @@
*/
package de.monticore.lang.monticar.cnnarch._cocos;
import de.monticore.lang.monticar.cnnarch._ast.ASTArchitecture;
import de.monticore.lang.monticar.cnnarch._symboltable.ArchitectureSymbol;
import de.monticore.lang.monticar.cnnarch._ast.ASTVariable;
import de.monticore.lang.monticar.cnnarch._symboltable.VariableSymbol;
import de.monticore.lang.monticar.cnnarch.helper.Constraints;
public class CheckVariableConstraints implements CNNArchASTArchitectureCoCo {
import java.util.HashSet;
import java.util.Set;
@Override
public class CheckVariableConstraints implements CNNArchASTVariableCoCo {
/*@Override
public void check(ASTArchitecture node) {
ArchitectureSymbol architecture = (ArchitectureSymbol) node.getSymbol().get();
}
}*/
/*Set<VariableSymbol> variableSet = new HashSet<>();
Set<VariableSymbol> variableSet = new HashSet<>();
@Override
public void check(ASTVariable node) {
VariableSymbol variable = (VariableSymbol) node.getSymbol().get();
if (!variableSet.contains(variable)) {
if (variable.getType() == VariableType.GLOBAL) {
if (variable.hasExpression() && variable.getExpression().isResolvable()) {
variable.getExpression().resolveOrError();
Constraint.check(variable);
}
if (variable.hasExpression() && variable.getExpression().isResolvable()) {
variable.getExpression().resolveOrError();
Constraints.check(variable);
}
}
variableSet.add(variable);
}*/
}
}
\ No newline at end of file
......@@ -20,7 +20,7 @@
*/
package de.monticore.lang.monticar.cnnarch._symboltable;
import de.monticore.lang.monticar.cnnarch.helper.Constraint;
import de.monticore.lang.monticar.cnnarch.helper.Constraints;
import de.monticore.lang.monticar.cnnarch.helper.PredefinedVariables;
import de.monticore.symboltable.CommonSymbol;
import de.monticore.symboltable.MutableScope;
......@@ -58,7 +58,6 @@ public class ArgumentSymbol extends CommonSymbol {
return (MethodLayerSymbol) getEnclosingScope().getSpanningSymbol().get();
}
public ArchExpressionSymbol getRhs() {
return rhs;
}
......@@ -93,7 +92,7 @@ public class ArgumentSymbol extends CommonSymbol {
public void set(){
if (getRhs().isSimpleValue()){
getRhs().resolveOrError();
Constraint.check(this);
Constraints.check(this);
getParameter().setExpression((ArchSimpleExpressionSymbol) getRhs());
}
else {
......
......@@ -22,13 +22,17 @@ package de.monticore.lang.monticar.cnnarch._symboltable;
import de.monticore.lang.math.math._ast.ASTMathExpression;
import de.monticore.lang.math.math._ast.ASTMathFalseExpression;
import de.monticore.lang.math.math._ast.ASTMathTrueExpression;
import de.monticore.lang.math.math._symboltable.MathSymbolTableCreator;
import de.monticore.lang.math.math._symboltable.expression.MathExpressionSymbol;
import de.monticore.lang.math.math._symboltable.expression.MathNameExpressionSymbol;
import de.monticore.lang.math.math._visitor.MathVisitor;
import de.monticore.lang.monticar.cnnarch._ast.*;
import de.monticore.lang.monticar.cnnarch._visitor.CNNArchInheritanceVisitor;
import de.monticore.lang.monticar.cnnarch._visitor.CNNArchVisitor;
import de.monticore.lang.monticar.cnnarch._visitor.CommonCNNArchDelegatorVisitor;
import de.monticore.lang.monticar.cnnarch.helper.Constraint;
import de.monticore.lang.monticar.cnnarch.helper.Constraints;
import de.monticore.lang.monticar.cnnarch.helper.PredefinedMethods;
import de.monticore.lang.monticar.cnnarch.helper.PredefinedVariables;
import de.monticore.lang.monticar.types2._ast.ASTType;
......@@ -60,9 +64,8 @@ public class CNNArchSymbolTableCreator extends de.monticore.symboltable.CommonSy
}
private void initSuperSTC(final ResolvingConfiguration resolvingConfig) {
this.mathSTC = new MathSymbolTableCreator(resolvingConfig, scopeStack);
this.mathSTC = new ModifiedMathSymbolTableCreator(resolvingConfig, scopeStack);
CommonCNNArchDelegatorVisitor visitor = new CommonCNNArchDelegatorVisitor();
visitor.set_de_monticore_lang_monticar_cnnarch__visitor_CNNArchVisitor(this);
visitor.set_de_monticore_lang_math_math__visitor_MathVisitor(mathSTC);
......@@ -84,6 +87,7 @@ public class CNNArchSymbolTableCreator extends de.monticore.symboltable.CommonSy
private CNNArchVisitor realThis = this;
@Override
public CNNArchVisitor getRealThis() {
return realThis;
}
......@@ -220,7 +224,7 @@ public class CNNArchSymbolTableCreator extends de.monticore.symboltable.CommonSy
.name(node.getName())
.type(VariableType.IOVARIABLE)
.defaultValue(defaultValue)
.constraints(Constraint.INTEGER, Constraint.POSITIVE)
.constraints(Constraints.INTEGER, Constraints.POSITIVE)
.build();
//addToScope(ArchSimpleExpressionSymbol.of(variable));
addToScopeAndLinkWithNode(variable, node);
......@@ -270,6 +274,12 @@ public class CNNArchSymbolTableCreator extends de.monticore.symboltable.CommonSy
addToScopeAndLinkWithNode(variable, node);
}
@Override
public void endVisit(ASTMathExpression node) {
boolean t = true;
boolean b = t;
}
@Override
public void visit(ASTArchSimpleExpression ast) {
ArchSimpleExpressionSymbol sym = new ArchSimpleExpressionSymbol();
......
/**
*
* ******************************************************************************
* MontiCAR Modeling Family, www.se-rwth.de
* Copyright (c) 2017, Software Engineering Group at RWTH Aachen,
* All rights reserved.
*
* This project is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this project. If not, see <http://www.gnu.org/licenses/>.
* *******************************************************************************
*/
package de.monticore.lang.monticar.cnnarch._symboltable;
import de.monticore.lang.math.math._ast.ASTMathFalseExpression;
import de.monticore.lang.math.math._ast.ASTMathTrueExpression;
import de.monticore.lang.math.math._symboltable.MathSymbolTableCreator;
import de.monticore.lang.math.math._symboltable.expression.MathNameExpressionSymbol;
import de.monticore.lang.monticar.cnnarch.helper.PredefinedVariables;
import de.monticore.symboltable.MutableScope;
import de.monticore.symboltable.ResolvingConfiguration;
import java.util.Deque;
public class ModifiedMathSymbolTableCreator extends MathSymbolTableCreator {
//todo: remove this class if MontiMath is fixed
public ModifiedMathSymbolTableCreator(ResolvingConfiguration resolverConfiguration, MutableScope enclosingScope) {
super(resolverConfiguration, enclosingScope);
}
public ModifiedMathSymbolTableCreator(ResolvingConfiguration resolvingConfig, Deque<MutableScope> scopeStack) {
super(resolvingConfig, scopeStack);
}
//todo: only here because montimath true expression does not work at the moment
@Override
public void endVisit(ASTMathTrueExpression node){
MathNameExpressionSymbol symbol = new MathNameExpressionSymbol(PredefinedVariables.TRUE_NAME);
addToScopeAndLinkWithNode(symbol, node);
}
//todo: only here because montimath false expression does not work at the moment
@Override
public void endVisit(ASTMathFalseExpression node){
MathNameExpressionSymbol symbol = new MathNameExpressionSymbol(PredefinedVariables.FALSE_NAME);
addToScopeAndLinkWithNode(symbol, node);
}
}
......@@ -20,7 +20,7 @@
*/
package de.monticore.lang.monticar.cnnarch._symboltable;
import de.monticore.lang.monticar.cnnarch.helper.Constraint;
import de.monticore.lang.monticar.cnnarch.helper.Constraints;
import de.monticore.symboltable.CommonSymbol;
import de.monticore.symboltable.MutableScope;
import de.monticore.symboltable.Symbol;
......@@ -37,7 +37,7 @@ public class VariableSymbol extends CommonSymbol {
private VariableType type;
private ArchSimpleExpressionSymbol defaultExpression = null; //Optional
private ArchSimpleExpressionSymbol currentExpression = null; //Optional
private Set<Constraint> constraints = new HashSet<>();
private Set<Constraints> constraints = new HashSet<>();
protected VariableSymbol(String name) {
......@@ -69,15 +69,15 @@ public class VariableSymbol extends CommonSymbol {
return Optional.ofNullable(currentExpression);
}
public Set<Constraint> getConstraints() {
public Set<Constraints> getConstraints() {
return constraints;
}
protected void setConstraints(Set<Constraint> constraints) {
protected void setConstraints(Set<Constraints> constraints) {
this.constraints = constraints;
}
public void addConstraint(Constraint... constraints) {
public void addConstraint(Constraints... constraints) {
for (int i = 0; i < constraints.length; i++){
getConstraints().add(constraints[i]);
}
......@@ -158,7 +158,7 @@ public class VariableSymbol extends CommonSymbol {
private VariableType type = VariableType.PARAMETER;
private ArchSimpleExpressionSymbol defaultValue = null;
private String name = null;
private Set<Constraint> constraints = new HashSet<>();
private Set<Constraints> constraints = new HashSet<>();
public Builder type(VariableType type){
this.type = type;
......@@ -195,7 +195,7 @@ public class VariableSymbol extends CommonSymbol {
return this;
}
public Builder constraints(Constraint... constraints){
public Builder constraints(Constraints... constraints){
this.constraints = new HashSet<>(Arrays.asList(constraints));
return this;
}
......
......@@ -29,7 +29,7 @@ import java.util.List;
import static de.monticore.lang.monticar.cnnarch.helper.ErrorCodes.ILLEGAL_ASSIGNMENT_CODE;
public enum Constraint {
public enum Constraints {
NUMBER {
@Override
public boolean check(ArchSimpleExpressionSymbol exp) {
......@@ -152,7 +152,7 @@ public enum Constraint {
abstract protected String msgString();
public static void check(VariableSymbol variable){
for (Constraint constraint : variable.getConstraints()) {
for (Constraints constraint : variable.getConstraints()) {
if (!constraint.check(variable.getExpression())){
Log.error("0"+ ILLEGAL_ASSIGNMENT_CODE +" Illegal assignment. The variable '"
+ variable.getName() +"' must be " + constraint.msgString() + "."
......@@ -165,7 +165,7 @@ public enum Constraint {
VariableSymbol variable = argument.getParameter();
for (List<ArchSimpleExpressionSymbol> expList : argument.getRhs().getElements().get()){
for (ArchSimpleExpressionSymbol exp : expList){
for (Constraint constraint : variable.getConstraints()) {
for (Constraints constraint : variable.getConstraints()) {
if (!constraint.check(exp)){
Log.error("0"+ ILLEGAL_ASSIGNMENT_CODE +" Illegal assignment. This parameter '"
+ variable.getName() +"' must be " + constraint.msgString() + "."
......
......@@ -110,11 +110,11 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(UNITS_NAME)
.constraints(Constraint.INTEGER, Constraint.POSITIVE)
.constraints(Constraints.INTEGER, Constraints.POSITIVE)
.build(),
new VariableSymbol.Builder()
.name(NOBIAS_NAME)
.constraints(Constraint.BOOLEAN)
.constraints(Constraints.BOOLEAN)
.defaultValue(false)
.build()
)
......@@ -132,20 +132,20 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(KERNEL_NAME)
.constraints(Constraint.INTEGER_TUPLE, Constraint.POSITIVE)
.constraints(Constraints.INTEGER_TUPLE, Constraints.POSITIVE)
.build(),
new VariableSymbol.Builder()
.name(CHANNELS_NAME)
.constraints(Constraint.INTEGER, Constraint.POSITIVE)
.constraints(Constraints.INTEGER, Constraints.POSITIVE)
.build(),
new VariableSymbol.Builder()
.name(STRIDE_NAME)
.constraints(Constraint.INTEGER_TUPLE, Constraint.POSITIVE)
.constraints(Constraints.INTEGER_TUPLE, Constraints.POSITIVE)
.defaultValue(Arrays.asList(1, 1))
.build(),
new VariableSymbol.Builder()
.name(NOBIAS_NAME)
.constraints(Constraint.BOOLEAN)
.constraints(Constraints.BOOLEAN)
.defaultValue(false)
.build()
)
......@@ -190,7 +190,7 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(P_NAME)
.constraints(Constraint.NUMBER, Constraint.BETWEEN_ZERO_AND_ONE)
.constraints(Constraints.NUMBER, Constraints.BETWEEN_ZERO_AND_ONE)
.defaultValue(0.5)
.build()
)
......@@ -204,16 +204,16 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(KERNEL_NAME)
.constraints(Constraint.INTEGER_TUPLE, Constraint.POSITIVE)
.constraints(Constraints.INTEGER_TUPLE, Constraints.POSITIVE)
.build(),
new VariableSymbol.Builder()
.name(STRIDE_NAME)
.constraints(Constraint.INTEGER_TUPLE, Constraint.POSITIVE)
.constraints(Constraints.INTEGER_TUPLE, Constraints.POSITIVE)
.defaultValue(Arrays.asList(1, 1))
.build(),
new VariableSymbol.Builder()
.name(GLOBAL_NAME)
.constraints(Constraint.BOOLEAN)
.constraints(Constraints.BOOLEAN)
.defaultValue(false)
.build()
)
......@@ -230,16 +230,16 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(KERNEL_NAME)
.constraints(Constraint.INTEGER_TUPLE, Constraint.POSITIVE)
.constraints(Constraints.INTEGER_TUPLE, Constraints.POSITIVE)
.build(),
new VariableSymbol.Builder()
.name(STRIDE_NAME)
.constraints(Constraint.INTEGER_TUPLE, Constraint.POSITIVE)
.constraints(Constraints.INTEGER_TUPLE, Constraints.POSITIVE)
.defaultValue(Arrays.asList(1, 1))
.build(),
new VariableSymbol.Builder()
.name(GLOBAL_NAME)
.constraints(Constraint.BOOLEAN)
.constraints(Constraints.BOOLEAN)
.defaultValue(false)
.build()
)
......@@ -256,21 +256,21 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(NSIZE_NAME)
.constraints(Constraint.INTEGER, Constraint.NON_NEGATIVE)
.constraints(Constraints.INTEGER, Constraints.NON_NEGATIVE)
.build(),
new VariableSymbol.Builder()
.name(KNORM_NAME)
.constraints(Constraint.NUMBER)
.constraints(Constraints.NUMBER)
.defaultValue(2)
.build(),
new VariableSymbol.Builder()
.name(ALPHA_NAME)
.constraints(Constraint.NUMBER)
.constraints(Constraints.NUMBER)
.defaultValue(0.0001)
.build(),
new VariableSymbol.Builder()
.name(BETA_NAME)
.constraints(Constraint.NUMBER)
.constraints(Constraints.NUMBER)
.defaultValue(0.75)
.build()
)
......@@ -284,12 +284,12 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(FIX_GAMMA_NAME)
.constraints(Constraint.BOOLEAN)
.constraints(Constraints.BOOLEAN)
.defaultValue(true)
.build(),
new VariableSymbol.Builder()
.name(AXIS_NAME)
.constraints(Constraint.INTEGER, Constraint.NON_NEGATIVE)
.constraints(Constraints.INTEGER, Constraints.NON_NEGATIVE)
.defaultValue(ShapeSymbol.CHANNEL_INDEX)
.build()
)
......@@ -303,11 +303,11 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(INDEX_NAME)
.constraints(Constraint.INTEGER, Constraint.NON_NEGATIVE)
.constraints(Constraints.INTEGER, Constraints.NON_NEGATIVE)
.build(),
new VariableSymbol.Builder()
.name(N_NAME)
.constraints(Constraint.INTEGER, Constraint.POSITIVE)
.constraints(Constraints.INTEGER, Constraints.POSITIVE)
.build()
)
.shapeFunction((inputShapes, method) -> splitShapeFunction(inputShapes.get(0), method))
......@@ -320,7 +320,7 @@ public class PredefinedMethods {
.parameters(
new VariableSymbol.Builder()
.name(INDEX_NAME)
.constraints(Constraint.INTEGER, Constraint.NON_NEGATIVE)
.constraints(Constraints.INTEGER, Constraints.NON_NEGATIVE)
.build()
)
.shapeFunction((inputShapes, method) ->
......
......@@ -35,7 +35,7 @@ public class PredefinedVariables {
return new VariableSymbol.Builder()
.name(IF_NAME)
.type(VariableType.PARAMETER)
.constraints(Constraint.BOOLEAN)
.constraints(Constraints.BOOLEAN)
.defaultValue(true)
.build();
}
......@@ -44,6 +44,7 @@ public class PredefinedVariables {
return new VariableSymbol.Builder()
.name(FOR_NAME)
.type(VariableType.PARAMETER)
.constraints(Constraints.INTEGER, Constraints.POSITIVE)
.defaultValue(1)
.build();
}
......@@ -52,6 +53,7 @@ public class PredefinedVariables {
return new VariableSymbol.Builder()
.name(CARDINALITY_NAME)
.type(VariableType.PARAMETER)
.constraints(Constraints.INTEGER, Constraints.POSITIVE)
.defaultValue(1)
.build();
}
......
......@@ -54,7 +54,7 @@ public class SymtabTest extends AbstractSymtabTest {
public void testFixedThreeInput(){
Scope symTab = createSymTab("src/test/resources/valid_tests");
ArchitectureSymbol a = symTab.<ArchitectureSymbol>resolve(
"ThreeInputCNN_M14_alternative",
"Fixed_ThreeInputCNN_M14",
ArchitectureSymbol.KIND).orElse(null);
assertNotNull(a);
a.resolve();
......
......@@ -79,10 +79,10 @@ public class AbstractCoCoTest extends AbstractSymtabTest {
if (Log.getFindings().isEmpty() && node.getSymbol().isPresent()){
ArchitectureSymbol architecture = ((ArchitectureSymbol)node.getSymbol().get());
architecture.resolve();
CNNArchPostResolveCocos.createChecker().checkAll(node);
if (architecture.isResolved()){
architecture.getBody().getOutputShapes();
}
CNNArchPostResolveCocos.createChecker().checkAll(node);
}
new ExpectedErrorInfo().checkOnlyExpectedPresent(Log.getFindings());
}
......@@ -101,10 +101,10 @@ public class AbstractCoCoTest extends AbstractSymtabTest {
if (Log.getFindings().isEmpty() && node.getSymbol().isPresent()){
ArchitectureSymbol architecture = ((ArchitectureSymbol)node.getSymbol().get());
architecture.resolve();
CNNArchPostResolveCocos.createChecker().checkAll(node);
if (architecture.isResolved()){
architecture.getBody().getOutputShapes();
}
CNNArchPostResolveCocos.createChecker().checkAll(node);
}
expectedErrors.checkExpectedPresent(Log.getFindings(), "Got no findings when checking all "
+ "cocos. Did you forget to add the new coco to MontiArcCocos?");
......@@ -116,10 +116,10 @@ public class AbstractCoCoTest extends AbstractSymtabTest {
if (Log.getFindings().isEmpty() && node.getSymbol().isPresent()){
ArchitectureSymbol architecture = ((ArchitectureSymbol)node.getSymbol().get());
architecture.resolve();
postResolveCocos.checkAll(node);
if (architecture.isResolved()){
architecture.getBody().getOutputShapes();
}
postResolveCocos.checkAll(node);
}
expectedErrors.checkOnlyExpectedPresent(Log.getFindings(), "Got no findings when checking only "
+ "the given coco. Did you pass an empty coco checker?");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment