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

Fatal Compilation Error: preprocessor directives, not placed on new line #725

Closed
aabs opened this issue Dec 16, 2024 · 14 comments · Fixed by #727
Closed

Fatal Compilation Error: preprocessor directives, not placed on new line #725

aabs opened this issue Dec 16, 2024 · 14 comments · Fixed by #727
Labels
bug Something isn't working

Comments

@aabs
Copy link

aabs commented Dec 16, 2024

Describe the bug

Getting errors after updating to Microsoft Visual Studio Professional 2022 (64-bit) - Preview
Version 17.13.0 Preview 2.0.

The error messages produced indicate malformed code generation in Vogen:

1>C:\dev\aabs\5th-related\ast-builder\src\ast-model\obj\Debug\net9.0\Vogen\Vogen.ValueObjectGenerator\ast_AssemblyName.g.cs(97,37,97,38): error CS1040: Preprocessor directives must appear as the first non-whitespace character on a line

The original code looks like this:

[ValueObject<string>]
[Instance("anonymous", "", "For things like in-memory assemblies etc")]
public partial struct AssemblyName;

The generated code contains this:

#pragma warning disable CS8767 // Nullability of reference types in type of parameter doesn't match implicitly implemented member because of nullability attributes.

        public static bool TryFrom( #if NETCOREAPP3_0_OR_GREATER
[global::System.Diagnostics.CodeAnalysis.NotNullWhen(true)] 
#endif
        System.String? value, #if NETCOREAPP3_0_OR_GREATER
 [global::System.Diagnostics.CodeAnalysis.MaybeNullWhen(false)] 
#endif
        out AssemblyName vo)
#pragma warning restore CS8767 // Nullability of reference types in type of parameter doesn't match implicitly implemented member because of nullability attributes.

        {
. . . snip
        }

Note the fragment TryFrom( #if NETCOREAPP which is invalid.

Steps to reproduce

Update VS to latest preview (17.13.0 Preview 2.0).
Rebuild the code. Build will fail with the above issue.

I tried updating to the latest version of the Nuget pkg (5.0.6) and restarting VS, but the code generated still broke.

Expected behaviour

The #if should be on a new line. Then all should be fine.

@aabs aabs added the bug Something isn't working label Dec 16, 2024
@danielcweber
Copy link
Contributor

danielcweber commented Dec 16, 2024

Temporary workaround: Fixate the SDK to a working version and disable rollforward in a global.json. This works for me:

{
  "sdk": {
    "version": "9.0.101",
    "rollForward": "disable"
  }
}

@aabs
Copy link
Author

aabs commented Dec 16, 2024

thanks for the suggestion. I tried that, and then forced a regeneration (after a VS restart) and the result still contained malformed C#.

@aabs
Copy link
Author

aabs commented Dec 16, 2024

I was able to temporarily suppress the generation of the TryFrom and TryParse code by adding the following to my decls, but I'm not sure what the downstream consequences would be.

[ValueObject<uint>
#if VOGEN_IS_BROKEN
    ( parsableForStrings: ParsableForStrings.GenerateNothing, 
      tryFromGeneration: TryFromGeneration.Omit, 
      parsableForPrimitives: ParsableForPrimitives.GenerateNothing )
#endif
]

@nabeelio
Copy link

nabeelio commented Dec 19, 2024

I'm running into the same issue. I put the SDK version back to 9.0.100, and I'll try to keep going backwards. I updated last night, and I think I was on quite an old version.

I tried @aabs suggestion, but that parseableForStrings option would break things (I think, trying it now)

[ValueObject<string>(
    tryFromGeneration: TryFromGeneration.Omit,
    parsableForPrimitives: ParsableForPrimitives.GenerateNothing
)]
public readonly partial struct PirepId;

It's generating:

        /// <summary>
        /// </summary>
        /// <returns>
        /// True if the value passes any validation (after running any optional normalization).
        /// </returns>
        public static global::System.Boolean TryParse(
#if NETCOREAPP3_0_OR_GREATER
[global::System.Diagnostics.CodeAnalysis.NotNullWhen(true)]
#endif
        global::System.String? s, global::System.IFormatProvider? provider,
#if NETCOREAPP3_0_OR_GREATER
[global::System.Diagnostics.CodeAnalysis.MaybeNullWhen(false)]
#endif
 out PirepId result)

@nabeelio
Copy link

Damn, looks like only back to 9.0.100 is available.

@nabeelio
Copy link

Adding parsableForStrings: ParsableForStrings.GenerateNothing did work, but not sure of the side-effects

danielcweber added a commit to danielcweber/Vogen that referenced this issue Dec 20, 2024
SteveDunn pushed a commit to danielcweber/Vogen that referenced this issue Dec 26, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…eMaybeNullWhenFalse. Fixes SteveDunn#725.
@SteveDunn
Copy link
Owner

SteveDunn commented Dec 26, 2024

Thanks for the report. I think the problem was, at some point, I wrapped the attribute with #if NETCOREAPP3_0_OR_GREATER; the attribute didn't need to be on a newline, but the preprocessor obviously does. Thanks for the fix and the suggestion @danielcweber . I'm just updating your PR with rebased snapshots (the trick, when changing anything that changes the output, is to just run .\RunSnapshots.ps1 -v "minimal" -reset (and then go on a short holiday while it resets 30,000+ files :/ )

SteveDunn added a commit that referenced this issue Dec 26, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Fix #725: Introduce line breaks in GenerateNotNullWhenTrueAttribute and GenerateMaybeNullWhenFalse
@Khaos66
Copy link

Khaos66 commented Dec 26, 2024

I just love how I stumbled across this issue just today and an hour later it's fixed <3 THX YOU ALL! AWESOME!!!

@SteveDunn
Copy link
Owner

Yes, this is fixed in 6.0.0-beta.3. I've created a Discord thread in the Roslyn channel as something has changed (but admittedly, the source that Vogen generated was also wrong, in that it needed preprocessors on their lines)
https://discord.com/channels/732297728826277939/735233259763400715/1321845291774906368

@SteveDunn
Copy link
Owner

For reference, this is the Roslyn bug report:

dotnet/roslyn#76566

@viceroypenguin
Copy link
Contributor

For reference, this is the Roslyn bug report:

dotnet/roslyn#76566

I will add that it has been, at least in conversations on the discord, the public recommendation that generated code should not be run through a formatter. This is because formatting is incredibly slow and source generators should generate code quickly. As such, it is suggested that generators generate good enough code and not use formatters at all.

@Khaos66
Copy link

Khaos66 commented Dec 26, 2024

https://learn.microsoft.com/en-us/dotnet/api/microsoft.entityframeworkcore.infrastructure.indentedstringbuilder?view=efcore-6.0

Not sure if you want a dependency on efcore but they're using a StringBuilder that supports indentation.
I've build one myself for my own srcgens

@SteveDunn
Copy link
Owner

Thanks @Khaos66 - I'll take a look

@SteveDunn
Copy link
Owner

For reference, this is the Roslyn bug report:
dotnet/roslyn#76566

I will add that it has been, at least in conversations on the discord, the public recommendation that generated code should not be run through a formatter. This is because formatting is incredibly slow and source generators should generate code quickly. As such, it is suggested that generators generate good enough code and not use formatters at all.

Thank you. Yes, I was in two minds whether to format or not as I anticipated some performance overhead. I really need to get around to profiling Vogen as I'm sure there's areas that could be optimised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
6 participants