Change ilasm command line arguments for PDB generation to match csc convention

[ivanpovazan]

Based on discussion from #5051 thread (which can be seen bellow) I am opening a new issue which can be referenced when the fix is available.

---

So the question is not about whether -pdbfmt and -debug options can be merged but whether it will be clear enough for ilasm users.

@ivanpovazan I would agree that taking inspiration from the csc options is appropriate here.

-debug[+|-]                   Emit debugging information
-debug:{full|pdbonly|portable|embedded}
                              Specify debugging type ('full' is default,
                              'portable' is a cross-platform format,
                              'embedded' is a cross-platform format embedded into
                              the target .dll or .exe)

Since the debug flag appears to have a predefined set of options for ilasm and I personally wouldn't want to deal with the matrix of multiple argument, I propose slightly modifying your suggestion to align with existing ilasm syntax and acknowledge csc convention.

-debugfmt={full|portable}

At this point we can support full and portable. In the future we can add embedded as the need arises.

Originally posted by @AaronRobinsonMSFT in #5051 (comment)

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ivanpovazan]

@AaronRobinsonMSFT @noahfalk @BruceForstall
I have opened this issue not to pollute the #5051 thread.

The only problem I can see with changing the -pdbfmt flag into -debugfmt is for the case when someone uses -pdb flag:

/PDB            Create the PDB file without enabling debug info tracking
/DEBUG          Disable JIT optimization, create PDB file, use sequence points from PDB
/DEBUG=IMPL     Disable JIT optimization, create PDB file, use implicit sequence points
/DEBUG=OPT      Enable JIT optimization, create PDB file, use implicit sequence points 

• With my proposal -pdbfmt there are no ambiguities:
  -pdb -pdbfmt=portable - disable debug info tracking but generate pdb in specified format
  -debug -pdbfmt=portable - enable debug info tracking (implies PDB generation) and generate pdb in specified format

• While with -debugfmt this becomes a bit unclear
  -pdb -debugfmt=portable

I can fix this without a problem, but would like to see what is the decision on this topic.

---

PS Keep in mind full PDB format is not supported only portable is .

[AaronRobinsonMSFT]

The only problem I can see with changing the -pdbfmt flag into -debugfmt is for the case when someone uses -pdb flag:

@ivanpovazan I think that is a reasonable argument. I am fine with the flag itself, but the flag argument should really match csc at the very least. That seems to change -pdbfmt=CLASSIC to -pdbfmt=FULL?

[ivanpovazan]

@AaronRobinsonMSFT yes, I am down to change the CLASSIC into FULL, but would like to check two things.

1. If I am not mistaken, I believe Roslyn compiler would in case of -debug:full produce the old "classic" PDB on Windows, while on other systems, this would actually mean producing PDBs in portable format.
   So, if we are trying to follow the convention, the same should apply to ilasm as well. Am I right?

2. The second thing I wanted to check is, whether it would be wise to set portable PDB as default for .net core ilasm, since currently it does not support the original "full" PDB format. This way there would be no need for potential changes in scripts/tools for people who use ilasm on daily basis.

[AaronRobinsonMSFT]

1. If I am not mistaken, I believe Roslyn compiler would in case of -debug:full produce the old "classic" PDB on Windows, while on other systems, this would actually mean producing PDBs in portable format.
   So, if we are trying to follow the convention, the same should apply to ilasm as well. Am I right?

Yes, the old PDB format is what 'full' means.

2. The second thing I wanted to check is, whether it would be wise to set portable PDB as default for .net core ilasm, since currently it does not support the original "full" PDB format. This way there would be no need for potential changes in scripts/tools for people who use ilasm on daily basis.

Yes. The default should remain. I was just implying the terminology should be the same, not suggesting we change the default values.

Thanks for confirming!

[ivanpovazan]

Based on #45979 I believe this isn't an issue anymore.
Now ILAsm generates only portable PDBs and there are no more issues with -pdbfmt command line argument.

@jkotas I think you can close this issue.