OpenDreamProject / OpenDream

A project for running games made in the DM programming language
MIT License
200 stars 109 forks source link

Preprocessor adds excess tabs after #if macros not followed by empty lines producing invalid code #1885

Closed Furrior closed 2 months ago

Furrior commented 2 months ago

See title. #ifdef works fine. Original discord message I used this command ODCompiler --suppress-unimplemented --dump-preprocessor --no-standard tgstation.dme and tg repo to reproduce.

This:

/datum/controller/subsystem/processing/station/Initialize()
    //If doing unit tests we don't do none of that trait shit ya know?
    // Autowiki also wants consistent outputs, for example making sure the vending machine page always reports the normal products
    #if !defined(UNIT_TESTS) && !defined(AUTOWIKI)
    SetupTraits()
    display_lobby_traits()
    #endif

    announcer = new announcer() //Initialize the station's announcer datum
    SSparallax.post_station_setup() //Apply station effects that parallax might have

    return SS_INIT_SUCCESS

Becomes this:

/datum/controller/subsystem/processing/station/Initialize()
        SetupTraits()
    display_lobby_traits()
    announcer = new announcer() 
    SSparallax.post_station_setup() 
    return 2

Workaround is to add an extra empty line:

/datum/controller/subsystem/processing/station/Initialize()
    //If doing unit tests we don't do none of that trait shit ya know?
    // Autowiki also wants consistent outputs, for example making sure the vending machine page always reports the normal products
    #if !defined(UNIT_TESTS) && !defined(AUTOWIKI)
+
    SetupTraits()
    display_lobby_traits()
    #endif

    announcer = new announcer() //Initialize the station's announcer datum
    SSparallax.post_station_setup() //Apply station effects that parallax might have

    return SS_INIT_SUCCESS

Resulting in correct code:

/datum/controller/subsystem/processing/station/Initialize()
    SetupTraits()
    display_lobby_traits()
    announcer = new announcer() 
    SSparallax.post_station_setup() 
    return 2

It also can insert more than one tab. Example:

/client/proc/send_resources()
#if (PRELOAD_RSC == 0)
    var/static/next_external_rsc = 0
    var/list/external_rsc_urls = CONFIG_GET(keyed_list/external_rsc_urls)
    if(length(external_rsc_urls))
        next_external_rsc = WRAP(next_external_rsc+1, 1, external_rsc_urls.len+1)
        preload_rsc = external_rsc_urls[next_external_rsc]
#endif

    spawn (10) //removing this spawn causes all clients to not get verbs. (this can't be addtimer because these assets may be needed before the mc inits)

        //load info on what assets the client has
        src << browse('code/modules/asset_cache/validate_assets.html', "window=asset_cache_browser")

        //Precache the client with all other assets slowly, so as to not block other browse() calls
        if (CONFIG_GET(flag/asset_simple_preload))
            addtimer(CALLBACK(SSassets.transport, TYPE_PROC_REF(/datum/asset_transport, send_assets_slow), src, SSassets.transport.preload), 5 SECONDS)

        #if (PRELOAD_RSC == 0)
        addtimer(CALLBACK(src, TYPE_PROC_REF(/client, preload_vox)), 1 MINUTES)
        #endif

becomes

/client/proc/send_resources()
    var/static/next_external_rsc = 0
    var/list/external_rsc_urls = global.config.Get(/datum/config_entry/keyed_list/external_rsc_urls)
    if(length(external_rsc_urls))
        next_external_rsc = clamp(( 1 == external_rsc_urls.len+1 ? 1 : (next_external_rsc+1) - (round(((next_external_rsc+1) - (1))/((external_rsc_urls.len+1) - (1))) * ((external_rsc_urls.len+1) - (1))) ),1,external_rsc_urls.len+1)
        preload_rsc = external_rsc_urls[next_external_rsc]
    spawn (10) 
        src << browse('code/modules/asset_cache/validate_assets.html', "window=asset_cache_browser")
        if (global.config.Get(/datum/config_entry/flag/asset_simple_preload))
            _addtimer(new /datum/callback(SSassets.transport, (nameof(/datum/asset_transport.proc/send_assets_slow)), src, SSassets.transport.preload),5 *10, file = "code/modules/client/client_procs.dm", line = 939)
                _addtimer(new /datum/callback(src, (nameof(/client.proc/preload_vox))),1 *10*60, file = "code/modules/client/client_procs.dm", line = 942)

Simple case to reproduce/use as an unit test

#define SOMEDEF1 1

/proc/RunTest()
#define SOMEDEF1 1

/proc/RunTest()
    var/a = 1

    if(0)
        . = . // NOOP
    #if SOMEDEF1
    a = 2
    #endif
    ASSERT(a == 2)
Furrior commented 2 months ago

Possible solution:

diff --git a/DMCompiler/Compiler/DMPreprocessor/DMPreprocessor.cs b/DMCompiler/Compiler/DMPreprocessor/DMPreprocessor.cs
index 0b77570f..1d9db66d 100644
--- a/DMCompiler/Compiler/DMPreprocessor/DMPreprocessor.cs
+++ b/DMCompiler/Compiler/DMPreprocessor/DMPreprocessor.cs
@@ -94,6 +94,7 @@ public sealed class DMPreprocessor(bool enableDirectives) : IEnumerable<Token> {
                     HandleUndefineDirective(token);
                     break;
                 case TokenType.DM_Preproc_If:
+                    _bufferedWhitespace.Clear();
                     HandleIfDirective(token);
                     break;
                 case TokenType.DM_Preproc_Ifdef: