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.

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

Fix https://github.com/ra3xdh/qucs_s/issues/967: #30

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

ThomasZecha
Copy link

-The lookahead-pattern for differentiate the 3-/4-/5-node spice-bjt
erroneously contains newline. This causes read a spice bjt-line don't
stop at new line and detect a 5-node bjt instead of 3-node bjt as
happen for AD822X.cir of Opamp_AC_Tran.zip from the issue 967.
This is fixed and tested against AD822X.cir. The critical lines from
AD822X.cir are included in the bjt.cir qucsconv_rf testing example.

cherry-picked from 3c375fd:

Fix build issues
-fixed sprintf format buffer overflow warning: check_citi.cpp,
check_mdl.cpp, check_spice.cpp
-fixed signed compare/expression warning: check_mdl.cpp, circuit.cpp,
hbsolver.cpp, e_trsolver.cpp, nasolver.cpp, spline.cpp, tmatrix.cpp,
tvector.cpp
-fixed clearing of non-trivial type via memset warning by static_cast:
circuit.cpp, tmatrix.cpp
-fixed int-in-bool-context warning: thyristor.cpp
-fixed unnecessary parentheses warning: environment.cpp, equation.cpp,
evaluate.cpp
-fixed case fallthrough warning: equation.cpp
-fixed unused parameter/variable warning: e_trsolver.cpp, real.cpp, qucs_action.cpp
-fixed infinite recursion warning: real.cpp, real.h
-fixed format type warning: module.cpp
-fixed depracated copy warning by adding assignment operator: strlist.cpp,
strlist.h
-fixed misleading indentation warning: vector.cpp
-fixed deprecated yacc/bison name-prefix directive warning: parse_spice.ypp,
parse_vcd.ypp, parse_citi.ypp, parse_csv.ypp, parse_dataset.ypp, parse_mdl.ypp
parse_mdl.ypp parse_netlist.ypp, parse_touchstone.ypp parse_zvr.ypp
-fixed useless yacc/bison symbol definition: parse_vcd.ypp
-fixed shift/reduce-conflicts (sr) and reduce/reduce-conflicts (rr) of the spice-parser
build by parse_spice.ypp and scan_spice.lpp:
-sr-conflicts solved by introducing token/rule precedence
-rr-conflicts solved by introducing token lookahead's in the spice scanner
-verification of equalness of generated output from both the converter before and
after the code change done with spice input bjt.cir done
-fixed shift/reduce-conflicts (sr) and reduce/reduce-conflicts (rr) of the csv-parser
build by parse_csv.ypp and scan_csv.lpp:
-sr/rr-conflicts solved by downsize and reorder grammatic rules
-verification of equalness of generated output from both the converter before and
after the code change done with csv input qucs_csv.csv done
-removed useless scanner token definition and replaced misleading operator assocativity
with precedence: parse_netlist.ypp
-fixed shift/reduce-conflicts (sr) of the touchstone-parser build by parse_touchstone.ypp and
scan_touchstonelpp:
-sr conflicts solved by remove useless/redundant grammatic rules
-verification of equalness of generated output from both the converter before and
after the code change done with touchstone input R50C1pF.s1p done

@ThomasZecha
Copy link
Author

@ra3xdh: Hi Vadim, it seems that the PR will not be accepted. May the the change set is to big, or the error risk is not acceptable? Is there something I can do?
From my point of view all the build issues shall be fixed soon or later, and this PR is a big step to reach it. I would go even further and switch to Werror after a warning-free build status.
Thanks.

@ra3xdh
Copy link
Owner

ra3xdh commented Oct 11, 2024

This PR may be accepted. Please split this patch at least into two parts:

  • The fixes related to qucsator simulator itself (C++ build warnings)
  • The fixes related to qucsconv flex/bison parsers

The qucsconv is not checked by automatic tests and it requires to test every combination by hands. See also #27. I will try to provide a review in the next weeks. Don't expect a quick response, because I am short in time in this month.

ThomasZecha added 8 commits October 22, 2024 12:54
-fixed sprintf format buffer overflow warning: check_citi.cpp,
 check_mdl.cpp, check_spice.cpp
-fixed signed compare/expression warning: check_mdl.cpp, circuit.cpp,
 hbsolver.cpp, e_trsolver.cpp, nasolver.cpp, spline.cpp, tmatrix.cpp,
 tvector.cpp, equation.cpp, sp_options.cpp
-fixed clearing of non-trivial type via memset warning by static_cast:
 circuit.cpp, tmatrix.cpp
-fixed int-in-bool-context warning: thyristor.cpp
-fixed unnecessary parentheses warning: environment.cpp, equation.cpp,
 evaluate.cpp
-fixed case fallthrough warning: equation.cpp
-fixed unused parameter/variable warning: e_trsolver.cpp
-fixed format type warning: module.cpp
-fixed depracated copy warning by adding assignment operator: strlist.cpp,
 strlist.h
-fixed misleading indentation warning: vector.cpp

Signed-off-by: ThomasZecha <[email protected]>
-increased bison diagnosis info output

Signed-off-by: ThomasZecha <[email protected]>
-fixed deprecated yacc/bison name-prefix directive warning: parse_citi.ypp, parse_dataset.ypp,
 parse_mdl.ypp parse_zvr.ypp

Signed-off-by: ThomasZecha <[email protected]>
 -fixed deprecated bison name-prefix directive warning and useless
  bison symbol definition: parse_vcd.ypp

Signed-off-by: ThomasZecha <[email protected]>
-fixed deprecated bison name-prefix directive warning, shift/reduce-conflicts (sr)
 and reduce/reduce-conflicts (rr) of the csv-parser build by parse_csv.ypp and scan_csv.lpp:
   -sr/rr-conflicts solved by downsize and reorder grammatic rules
   -verification of equalness of generated output from both the converter before and
    after the code change done with csv input qucs_csv.csv done

Signed-off-by: ThomasZecha <[email protected]>
-fixed deprecated bison name-prefix directive warning and shift/reduce-conflicts (sr)
 of the touchstone-parser build by parse_touchstone.ypp and scan_touchstone.lpp:
   -sr conflicts solved by remove useless/redundant grammatic rules
   -verification of equalness of generated output from both the converter before and
    after the code change done with touchstone input R50C1pF.s1p done

Signed-off-by: ThomasZecha <[email protected]>
-fixed deprecated bison name-prefix directive warning and removed useless scanner token
 definition and replaced misleading operator assocativity with precedence: parse_netlist.ypp

Signed-off-by: ThomasZecha <[email protected]>
-fixed deprecated yacc/bison name-prefix directive warning, shift/reduce-conflicts (sr) and
 reduce/reduce-conflicts (rr) of the spice-parser build by parse_spice.ypp and scan_spice.lpp:
   -sr-conflicts solved by introducing token/rule precedence
   -rr-conflicts solved by introducing token lookahead's in the spice scanner
   -verification of equalness of generated output from both the converter before and
    after the code change done with spice input bjt.cir done
-fixed ra3xdh/qucs_s#967:
   -The lookahead-pattern for differentiate the 3-/4-/5-node spice-bjt
    erroneously contains newline. This causes read a spice bjt-line don't
    stop at new line and detect a 5-node bjt instead of 3-node bjt as
    happen for AD822X.cir of Opamp_AC_Tran.zip from the issue 967.
    This is fixed and tested against AD822X.cir. The critical lines from
    AD822X.cir are included in the bjt.cir qucsconv_rf testing example.

Signed-off-by: ThomasZecha <[email protected]>
@ThomasZecha
Copy link
Author

Change-set splitted now in 1 c++-part commit covering all build-issue fixes for c++ source code and 7 flex/bison part commits dedicated to the different changed parsers and general topics for fix flex/bison build issues.

@ra3xdh
Copy link
Owner

ra3xdh commented Oct 29, 2024

I have seen the spitted PR. I will provide a review within the next month.

@ThomasZecha
Copy link
Author

Any blocking points?

@ra3xdh
Copy link
Owner

ra3xdh commented Dec 23, 2024

No blocking points. The qucsconv related part requires a manual testing. I am time constrained now and try to perform this task in January. You may speed up the merging if you provide a patch implementing an automatic test to qucs-test suite. See #27

@ra3xdh ra3xdh linked an issue Dec 23, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix C++ warnings
2 participants