Skip to content
Snippets Groups Projects

vkd3d-shader: Support namespaces in preproc macro identifiers.

Merged Arek Hiler requested to merge ivyl/vkd3d:preproc-colons into master

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Arek Hiler added 3 commits

    added 3 commits

    • 93e47257 - tests/shader_runner: Rename [preproc fail] to [preproc invalid].
    • bec64314 - tests/shader_runner: Add [preproc fail] for when 'fail' is present after processing.
    • eb0ea71b - vkd3d-shader: Support namespaces in preproc macro identifiers.

    Compare with previous version

  • Arek Hiler added 15 commits

    added 15 commits

    • eb0ea71b...2ac7f650 - 12 commits from branch wine:master
    • 32f3f79e - tests/shader_runner: Rename [preproc fail] to [preproc invalid].
    • e6fd949e - tests/shader_runner: Add [preproc fail] for when 'fail' is present after processing.
    • e86a4607 - vkd3d-shader: Support namespaces in preproc macro identifiers.

    Compare with previous version

  • I don't mind 1/3 regardless, but in the case of 2/3, usually the approach has simply been to write the test slightly differently. Put another way, if you're going to test that "fail" is in there you might as well just replace it with "pass" in the source. In this case we could have

    [preproc]
    #define Key1:pass value
    Key1:pass
    
    [preproc]
    #define Key1::::pass value
    Key1::::pass
    
    [preproc]
    #define Key::(x) value
    Key::(pass)

    Note also that there is test_preprocess() in hlsl_d3d12.c if we need something that the pass/fail bit alone isn't expressive enough for.

    Anyway, that aside, there's other interesting test cases I'd like to see:

    • Can you substitute half the namespace? E.g.
    #define foo bar
    #define qux::bar fail
    #define bar::qux fail
    qux::foo
    foo::qux

    A quick test says no, which is unsurprising, but I'd like to have that in the suite.

    • Can you put spaces before or after the colons? (I don't know offhand if C++ lets you do this, but it seems like a reasonable thing to test.)

    • Your implementation allows tokens like "foo::1bar". Is this correct?

    • {IDENTIFIER} is used for preprocessor directives too. Is this correct? E.g. is "#define::foo bar" an invalid directive, or does it define ::foo?

    • Can you use namespaced identifiers as macro arguments?

    Also, I don't think you need to escape colons in flex, or am I mistaken?

  • Arek Hiler added 1 commit

    added 1 commit

    • 06cb058f - vkd3d-shader: Support namespaces in preproc macro identifiers.

    Compare with previous version

  • Author Contributor

    Thanks for the review!

    Can you put spaces before or after the colons? (I don't know offhand if C++ lets you do this, but it seems like a reasonable thing to test.)

    I'm not sure what you mean by that. Everything else should be addressed by the new revision :-)

  • Arek Hiler added 1 commit

    added 1 commit

    • 2cfdebe7 - vkd3d-shader: Support namespaces in preproc macro identifiers.

    Compare with previous version

  • Author Contributor

    <strfllw> ivyl: I think zf is asking what things like "#define foo:: bar pass" and "#define foo ::bar pass" do

    I've added those

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet added 25 commits

    added 25 commits

    Compare with previous version

Please register or sign in to reply
Loading