antlr4 icon indicating copy to clipboard operation
antlr4 copied to clipboard

Split ctorAttrs from attrs in codegen model StructDecl

Open akosthekiss opened this issue 2 years ago • 4 comments
trafficstars

This helps avoid dead code in ctorAttr initialization in some script targets (namely Python3 and JavaScript).

akosthekiss avatar Oct 21 '23 20:10 akosthekiss

I've created an example grammar:

grammar Attrs;

start
  returns [int r=0]
  locals [int l=1]
  : attr_a=a[1] attr_bc=bc[2, 3]
  ;

a[int x]
  returns [s:int=2]
  locals [m:int=3]
  : attr_A=A;

bc[y:int, z:int]
  returns [int t]
  locals [int n]
  : attr_B=B # b
  | attr_C=C # c
  ;

A: 'a';
B: 'b';
C: 'c';

And created a simple diff utility to check what difference the commit makes on all the supported targets:

ANTLR_DEV_DIR=~/devel/antlr4
ANTLR_JAR=~/.m2/repository/org/antlr/antlr4/4.13.2-SNAPSHOT/antlr4-4.13.2-SNAPSHOT-complete.jar
ATTRS_G4_DIR=$(pwd)

function generate { # $1:branch $2:dir
  cd $ANTLR_DEV_DIR
  git checkout $1
  mvn install -DskipTests=true

  cd $ATTRS_G4_DIR
  rm -rf $2
  for TARGET_LANG in Cpp CSharp Dart Go Java JavaScript PHP Python3 Swift TypeScript; do
    java -jar $ANTLR_JAR Attrs.g4 -no-listener -Dlanguage=$TARGET_LANG -o $2
  done
}

generate dev a
generate attr b
diff -r a b -C 5

The output is:

diff -r -C 5 a/AttrsParser.js b/AttrsParser.js
*** a/AttrsParser.js	Sat Oct 21 22:45:01 2023
--- b/AttrsParser.js	Sat Oct 21 22:45:14 2023
***************
*** 167,177 ****
              invokingState = -1;
          }
          super(parent, invokingState);
          this.parser = parser;
          this.ruleIndex = AttrsParser.RULE_a;
-         this.x = null
          this.s = 2
          this.m = 3
          this.attr_A = null;
          this.x = x || null;
      }
--- 167,176 ----
***************
*** 195,206 ****
              invokingState = -1;
          }
          super(parent, invokingState);
          this.parser = parser;
          this.ruleIndex = AttrsParser.RULE_bc;
-         this.y = null
-         this.z = null
          this.t = null
          this.n = null
          this.y = y || null;
          this.z = z || null;
      }
--- 194,203 ----
diff -r -C 5 a/AttrsParser.py b/AttrsParser.py
*** a/AttrsParser.py	Sat Oct 21 22:45:02 2023
--- b/AttrsParser.py	Sat Oct 21 22:45:15 2023
***************
*** 99,109 ****
          __slots__ = 'parser'
  
          def __init__(self, parser, parent:ParserRuleContext=None, invokingState:int=-1, x:int=None):
              super().__init__(parent, invokingState)
              self.parser = parser
-             self.x = None
              self.s = 2
              self.m = 3
              self.attr_A = None # Token
              self.x = x
  
--- 99,108 ----
***************
*** 137,148 ****
          __slots__ = 'parser'
  
          def __init__(self, parser, parent:ParserRuleContext=None, invokingState:int=-1, y:int=None, z:int=None):
              super().__init__(parent, invokingState)
              self.parser = parser
-             self.y = None
-             self.z = None
              self.t = None
              self.n = None
              self.y = y
              self.z = z
  
--- 136,145 ----

That is, the commit only makes the superfluous assignments go away in generated Python3 and JavaScript code. (I hope I managed to cover all the cases.)

akosthekiss avatar Oct 21 '23 20:10 akosthekiss

Is this too big a change proposed for too little gain?

akosthekiss avatar Nov 08 '23 09:11 akosthekiss

@parrt this seems to make a lot of sense to me

ericvergnaud avatar Jul 28 '24 20:07 ericvergnaud

oh man...i don't have to really think about this one. and as you say "big" change.

parrt avatar Jul 28 '24 21:07 parrt