Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Converter from PCM to DFD #9

Merged
merged 38 commits into from
Sep 9, 2024
Merged

Add Converter from PCM to DFD #9

merged 38 commits into from
Sep 9, 2024

Conversation

Entenwilli
Copy link
Member

This PR adds a complete converter from PCM models to DFD models

01Parzival10 and others added 16 commits June 24, 2024 14:34
@01Parzival10
Feat: Changing from forwarding to normal Assignments
c47a786
@01Parzival10
Adjusting tests
2aa173f
@01Parzival10
WIP
4676f14
@01Parzival10
Feat: Redoing PCM2DFD Transformation
eed137b
@01Parzival10
Feat: Adding @Entenwilli 's PCM Behaviour Converter, currently non fu…
3905ed7 
…nctioning
@Entenwilli
fix(pcm): fix empty forwarding assignments
f33c94d
@01Parzival10
Fix: Handle incoming Return Variable with differently named outputVar…
d238dbb 
…iable, add logic for start Notes. TERMS ARE STILL WRONG!!
@01Parzival10
Feat: Merge main into branch
1159f3f
@Entenwilli
fix(pcm-behaviour): fix VariableName.Type.* assignments
05d090c
@Entenwilli
fix(pcm-behaviour): fix missing term for non referencing assignments
c3d0178
@01Parzival10
Fix: Handling scenario with Incoming but no outgoing Return
6dec99d
@01Parzival10
WIP
8d0b985
@Entenwilli
fix(pcm-converter): fix issues with pins and add logging
b014328
Removing recursive logic
6a5e10e
@Entenwilli
fix(pcm): prevent forwarding assignment of pcm elements
d189ed0
@Entenwilli
fix(pcm): fix guard of empty forwarding
9163a92
@Entenwilli Entenwilli added the enhancement New feature or request label Jul 16, 2024
@Entenwilli Entenwilli marked this pull request as draft July 16, 2024 12:34
@Entenwilli
Copy link
Member Author

@01Parzival10 Done the Renaming in DataFlowAnalysis/DataFlowAnalysis#190

@01Parzival10 01Parzival10 marked this pull request as ready for review July 26, 2024 12:32
@sebinside
Copy link
Member

sebinside commented Jul 30, 2024 •
edited by Entenwilli
Loading

Tasks:

  • make behavior converter readable
  • add test: same violations if PCM -> TFG vs. PCM -> DFD -> TFG
  • add test: grap a specific DFD node and manually compare the behavior against stoex

01Parzival10 and others added 2 commits July 30, 2024 13:39
@01Parzival10
Fix: Remove hardcoded path
a8c13a0
@01Parzival10
Feat: Adding Dummy Assignment to preserve Control flow and adding con…
7c8726b 
…straint based Tests, works in conjunction with dfdEfficiency branch
@01Parzival10
Copy link
Contributor

01Parzival10 commented Aug 16, 2024 •
edited
Loading

Currently, the pipeline fails due to crashing resulted from insufficient heap space

And it will continue to do so until #198 of the Analysis is reviewed and merged

@Entenwilli
Copy link
Member Author

PR is merged now, updatesite should update today

@Entenwilli
Copy link
Member Author

This PR is now ready for review, @sebinside @Nicolas-Boltz @uuqjz

uuqjz
uuqjz requested changes Aug 20, 2024
View reviewed changes
Copy link
Collaborator

@uuqjz uuqjz left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some stuff mostly regarding testing and some oversights
As i hopped between files the comments are not in file order

.project Outdated Show resolved Hide resolved
tests/org.dataflowanalysis.converter.testmodels/.project Outdated Show resolved Hide resolved
...s/org.dataflowanalysis.converter.tests/src/org/dataflowanalysis/converter/tests/PCMTest.java Show resolved Hide resolved
...s/org.dataflowanalysis.converter.tests/src/org/dataflowanalysis/converter/tests/PCMTest.java Show resolved Hide resolved
bundles/org.dataflowanalysis.converter/src/org/dataflowanalysis/converter/PCMConverter.java Outdated Show resolved Hide resolved
...s/org.dataflowanalysis.converter.tests/src/org/dataflowanalysis/converter/tests/PCMTest.java Show resolved Hide resolved
bundles/org.dataflowanalysis.converter/src/org/dataflowanalysis/converter/PCMConverter.java Outdated Show resolved Hide resolved
bundles/org.dataflowanalysis.converter/src/org/dataflowanalysis/converter/PCMConverter.java Outdated Show resolved Hide resolved
bundles/org.dataflowanalysis.converter/src/org/dataflowanalysis/converter/PCMConverter.java Outdated Show resolved Hide resolved
bundles/org.dataflowanalysis.converter/src/org/dataflowanalysis/converter/PCMConverter.java Outdated Show resolved Hide resolved
@Entenwilli
Copy link
Member Author

@uuqjz This PR is ready for re-review!

@Entenwilli Entenwilli requested a review from uuqjz August 31, 2024 13:57
uuqjz
uuqjz reviewed Aug 31, 2024
View reviewed changes
Copy link
Collaborator

@uuqjz uuqjz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

uuqjz
uuqjz previously approved these changes Aug 31, 2024
View reviewed changes
@Entenwilli Entenwilli requested a review from uuqjz August 31, 2024 14:46
uuqjz
uuqjz approved these changes Aug 31, 2024
View reviewed changes
Copy link
Collaborator

@uuqjz uuqjz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@uuqjz
Copy link
Collaborator

uuqjz commented Sep 3, 2024

@sebinside @Nicolas-Boltz, please review at your earliest convenience

sebinside
sebinside approved these changes Sep 6, 2024
View reviewed changes
Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the PCMConverter class is nearly 1000 lines long underlines its complexity - maybe that's something to refactor in a future version. For now, I'm impressed, the PR ist fine from my side.

Nicolas-Boltz
Nicolas-Boltz approved these changes Sep 9, 2024
View reviewed changes
Copy link
Member

@Nicolas-Boltz Nicolas-Boltz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me and can be merged for the 3.0.0 release.

However, the code should be refactored to improve some of the very complex and long methods, missing documentation and general length of the PCMConverter class.
I have created DataFlowAnalysis/DataFlowAnalysis#203 that covers these points as improvements for a future release.

@Nicolas-Boltz Nicolas-Boltz merged commit 03b0743 into main Sep 9, 2024
1 check passed
@Entenwilli Entenwilli mentioned this pull request Sep 10, 2024
Closed
@Nicolas-Boltz Nicolas-Boltz deleted the assignmentAndBehaviourChange branch September 24, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebinside sebinside sebinside approved these changes

@Nicolas-Boltz Nicolas-Boltz Nicolas-Boltz approved these changes

@uuqjz uuqjz uuqjz approved these changes

Assignees

@Entenwilli Entenwilli

@01Parzival10 01Parzival10

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Entenwilli @sebinside @01Parzival10 @uuqjz @Nicolas-Boltz