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

Minor fixups to provide better diagnostics and notes on what should be done. #47

Merged
merged 2 commits into from May 12, 2019

Conversation

Projects
None yet
2 participants
@tannergooding
Copy link
Member

commented May 12, 2019

This does some minor cleanup in the writer to ensure we are processing things like attributes correctly and adds some notes on where we could be doing better.

This also updates the main program to correctly print diagnostics and skip processing on error.


if (translationUnitError != CXErrorCode.CXError_Success)
{
Console.WriteLine($"Error: '{translationUnitError}' for '{file}'.");
var numDiagnostics = translationUnit.NumDiagnostics;
Console.WriteLine($"Error: Parsing failed for '{file}' due to '{translationUnitError}'.");

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 12, 2019

Author Member

If the parsing failed, there is no guarantee the translation unit is even valid to get diagnostics

@tannergooding tannergooding force-pushed the tannergooding:master branch from 22caf72 to 2d56e0f May 12, 2019

}
else if (translationUnit.NumDiagnostics != 0)
{
Console.WriteLine($"Diagnostics for '{file}':");

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 12, 2019

Author Member

A switch to not print warnings or to not skip on error could probably be provided in the future. For now though, clang and llvm generation both occur without any diagnostics and providing diagnostics for other files will help with resolving any codegen issues.

Console.Write(" ");
Console.WriteLine(diagnostic.Format(CXDiagnosticDisplayOptions.CXDiagnostic_DisplayOption).ToString());

skipProcessing |= (diagnostic.Severity == CXDiagnosticSeverity.CXDiagnostic_Error);

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 12, 2019

Author Member

Skipping processing on error/fatal diagnostics seems like the right default. Otherwise, you can end up with weird codegen due to missing types or clang misinterpreting a macro expansion as something else.

{
Debug.Assert(cursor.Kind == CXCursorKind.CXCursor_DLLExport);

if (_attachedData.TryGetValue(parent, out var data) && (data is AttachedFunctionDeclData functionDeclData))

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 12, 2019

Author Member

A dll export probably means you are processing the header with the wrong define. It might be useful to expose a way to surface diagnostics from our own processing up to the user...


if (_attachedData.TryGetValue(parent, out var data) && (data is AttachedFunctionDeclData functionDeclData))
{
functionDeclData.IsDllImport = true;

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 12, 2019

Author Member

We should probably look for this as a requirement to emitting the method in the generated code. If it isn't there, then it might have a method body (for example) and it won't resolve at runtime.

return false;
}

functionDeclData.IsDeprecated = true;

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 12, 2019

Author Member

ClangSharp currently has just one of these and it would be nice to surface correctly. Maybe via the [Obsolete] attribute...

var arr = new string[]
{
"-xc++", // The input files are C++
"-Wno-pragma-once-outside-header" // We are processing files which may be header files

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 12, 2019

Author Member

This is the one warning I think is common enough to always suppress.

Windows header files in general (e.g those from the Windows SDK) have a few other common warnings that it might be nice to provide a switch for easy suppression (such as #include directives using non matching casing).

@tannergooding tannergooding force-pushed the tannergooding:master branch from 2d56e0f to 915449d May 12, 2019

@tannergooding tannergooding merged commit 7c45035 into microsoft:master May 12, 2019

1 check passed

license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.