1. SCons
  2. Core
  3. SCons
  4. Pull requests

Pull requests

#380 Open
Repository
ivankravets
Branch
default
Repository
scons
Branch
default

Improvements for SCons C/C++ Pre-Processor

Bitbucket cannot automatically merge this request.

The commits that make up this pull request have been removed.

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

hg update default
hg pull -r default https://bitbucket.org/ivankravets/scons
hg merge a5f38f2f1d43
hg commit -m 'Merged in ivankravets/scons (pull request #380)'
Author
  1. Ivan Kravets
Reviewers
Description
  • New API for C.CScanner(advanced=False):

    • "Basic" C Scanner (default, the same behavior as before). Takes into account the type of bracketing used to include the file, and uses classic CPP rules for searching for the files based on the bracketing.
    • "Advanced". Interprets C/C++ Preprocessor conditional syntax ( #ifdef, #if, #else, #elif, #define, etc.). Compatible with Scons.Node.Node::get_implicit_deps and recursive behavior.
  • Improvements for SCons C/C++ Pre-Processor:

    • Handle UNSIGNED LONG and LONG numeric constants in DEC (keep support for HEX)
    • Evaluate "defined" chain #define MACRO ( ! (defined (A) || defined (B) )
    • Skip unrecognized directives, such as #if( defined …)
    • Ignore #include directive that depends on dynamic macrowhich is not located in state TABLE.
    • Cleanup CPP Expressions before evaluating

Comments (13)

  1. William Deegan

    Better to have a new (improved) set of code as single pull request. Thanks. Can you create some tests which fail prior to your changes,and pass after? Looks like it shouldn't be too hard to do.

    SCons policy requires (some) test coverage.

      1. Ivan Kravets author

        Thanks, will write tests later. I've been working on this PR since first commit. The code is obsolate according to new SCons.Node API. I see that last time when it was used is 2008 year :)

        I carefully retested under dozens different projects with multiple conditional syntax. Here is good project that uses PlatformIO as build system for embedded. They have different examples that depend on external libraries. PlatformIO builds Dependency Graph on-the-fly and tries to find all dependencies automatically. For example, https://travis-ci.org/platformio/platform-nordicnrf51/jobs/179539573#L281

        I use Basic C Scanner in the last stable release. However, there are a few complaints that PlatformIO handles dependencies that are hidden under C macro. That is why I've decided to reanimate this SCons Python-based C Pre Processor. I like this idea and would be glad if I can contribute to this amazing SCons project.

        P.S: I can't imagine PlatformIO without SCons. SCons is a heart of PlatformIO Build System. We have high-level API over SCons and allow developers to write own development platforms. For example, development platform for Espressif 8266. P.S.S: I'm new contributor to SCons Project, please correct me if I do some wrong. Thanks!

    1. Ivan Kravets author

      I finished it and released with PlatformIO 3.2 2 weeks ago. I used 2.5.1+this patch. Currently, a thousand developers use it and I didn't receive any bug reports according to new changes.

      Also, I put it to try/catch block. If "advanced" scanner fails, the classic will be used.

      In any case, this PR doesn't affect previous SCons releases/projects. I kept default behavior of def CScanner(advanced=False). It means that it should not break existing projects. I use 2 scanners in combination:

      CLASSIC_SCANNER = SCons.Scanner.C.CScanner()
      ADVANCED_SCANNER = SCons.Scanner.C.CScanner(advanced=True)
      
      1. William Deegan

        We'll need a couple things to pull this into the default branch:

        1. tests
        2. Doc changes with info about the new scanner?
        3. Are you seeing any notable difference in SCons performance with the scanner changes?
      2. Sye van der Veen

        Boolean-only arguments tend to muddy APIs, and calling this "advanced" will seem antiquated if-and-when a v3 of this scanner is created. What about creating this as a new class, perhaps CConditionalScanner, CMacroScanner, or CIfdefScanner?

        1. Ivan Kravets author

          I also thought on that before but we have different collisions on the names here:

          1. CScanner() means here Scanner based on C Preprocessor or...?. We don't see any remarks about "classic" or etc. It will mess developers.
          2. I don't know why, but Classic CScanner is implemented in global scope here: https://bitbucket.org/scons/scons/src/cf05517e78d4045ea767f59cd0daa335612b5926/src/engine/SCons/Scanner/init.py?at=default&fileviewer=file-view-default#init.py-385
          3. I haven't had an aim significantly change API. The core idea was to reanimate this cool feature with native C Preprocessor that was implemented in 2008 and later abandoned.

          If William doesn't have any objections according to API changes, I recommend the next:

          1. Move class ClassicCPP(Classic): from SCons / src / engine / SCons / Scanner / __init__.py to SCons / src / engine / SCons / Scanner / C.py
          2. Create new API for classic C Scanner named CClassicScanner(), which will behave as the current CScanner().
          3. Create new API for advanced C Scanner that is based on SCons / src / engine / SCons / cpp.py / PreProcessor named CPreProcessorScanner().
          4. Mark CScanner() as deprecated and return by default CClassicScanner().
            1. Ivan Kravets author

              We can import ClassicCPP from SCons.Scanner.C in SCons / src / engine / SCons / Scanner / __init__.py and keep compatible previous projects that depend on this class.

  2. William Deegan

    We definitely need some tests here. Ideally the tests would fail with the current cscanner, and pass with the new. This will ensure than no changes going forward break the new scanner and also it would serve to highlight the improvements over the old scanner.

    SCons does have a formal deprecation cycle. I'd like to stick with that for these kind of changes as well:

    https://bitbucket.org/scons/scons/wiki/DeprecationCycle

    Do you have any benchmarking #'s to compare the speed of the new vs current cscanner? Since this is widely used any change with slower performance could have negatively affect many users.

    Also we'll need doc changes to explain the difference and how to enable/disable the new scanner.