Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

[507077] Do not emit empty concatenation #69

Merged
merged 1 commit into from Dec 19, 2016
Merged

[507077] Do not emit empty concatenation #69

merged 1 commit into from Dec 19, 2016

Conversation

rovarga
Copy link
Contributor

@rovarga rovarga commented Nov 4, 2016

Java code generated from Xtend routinely contains output
like:

public CharSequence packageDefinition() {
StringConcatenation _builder = new StringConcatenation();
_builder.append("package ");
String _packageName = this.type.getPackageName();
_builder.append(_packageName, "");
_builder.append(";");
return _builder;
}

While StringConcatenation will detect the fact the indentation
string is empty, it is at the cost of runtime performance.

Before emitting the call to StringConcatenation, check if our
literal would end up being an empty string and if it would,
call the no-indentation append() variant instead.

Signed-off-by: Robert Varga nite@hq.sk

@eclipsewebmaster
Copy link

Issue tracker reference:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=507077

@svenefftinge
Copy link
Member

Have you executed the tests? I would assume that some test expectations would need to be updated for this, too.

@rovarga
Copy link
Contributor Author

rovarga commented Nov 6, 2016

Unfortunately I was unable to do so -- gradle built only part of the project and maven build failed on what I assume is wrong settings.xml. Can you point me at a howto?

@spoenemann
Copy link
Member

You can see in Jenkinsfile what Maven arguments are used by the server to build and test.

I pushed your branch to this repo so the tests are run automatically:
http://services.typefox.io/open-source/jenkins/job/xtext-xtend/job/rovarga_emptyIndent/

@spoenemann
Copy link
Member

The commit needs to be rebased onto master so the build works properly.

Here are the test results:
http://services.typefox.io/open-source/jenkins/job/xtext-xtend/job/rovarga_emptyIndent/lastCompletedBuild/testReport/

@rovarga
Copy link
Contributor Author

rovarga commented Dec 13, 2016

@spoenemann Thanks, I have rebased the patch for now. That Jenkinsfile helped, I am running the tests now and will post a fixed up version later today,

Java code generated from Xtend routinely contains output
like:

  public CharSequence packageDefinition() {
    StringConcatenation _builder = new StringConcatenation();
    _builder.append("package ");
    String _packageName = this.type.getPackageName();
    _builder.append(_packageName, "");
    _builder.append(";");
    return _builder;
  }

While StringConcatenation will detect the fact the indentation
string is empty, it is at the cost of runtime performance.

Before emitting the call to StringConcatenation, check if our
literal would end up being an empty string and if it would,
call the no-indentation append() variant instead.

Signed-off-by: Robert Varga <nite@hq.sk>
@rovarga
Copy link
Contributor Author

rovarga commented Dec 14, 2016

Should be ready to go, as far as I can tell (later tests seem to have massive memory requirements, hence failed on OOM for me).

@spoenemann
Copy link
Member

Thanks!

@spoenemann spoenemann merged commit 7e90e39 into eclipse:master Dec 19, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants