Skip to content

Invalid code for bool? to bool conversion in return #117

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

Closed
bahusoid opened this issue Dec 23, 2018 · 1 comment
Closed

Invalid code for bool? to bool conversion in return #117

bahusoid opened this issue Dec 23, 2018 · 1 comment
Labels
Milestone

Comments

@bahusoid
Copy link

bahusoid commented Dec 23, 2018

If you change the code below:
https://github.com/nhibernate/nhibernate-core/blob/4d887d8796e7c4e56cf88f52b5748826d6255c18/src/NHibernate/Engine/ISessionImplementor.cs#L47-L50

to the following:

internal static bool AutoFlushIfRequired(this ISessionImplementor implementor, ISet<string> querySpaces)
{
	return (implementor as AbstractSessionImpl)?.AutoFlushIfRequired(querySpaces) ?? false;
}

Iinvalid code is generated:

internal static async Task<bool> AutoFlushIfRequiredAsync(this ISessionImplementor implementor, ISet<string> querySpaces, CancellationToken cancellationToken)
{
	cancellationToken.ThrowIfCancellationRequested();
	var autoFlushIfRequiredTask = (implementor as AbstractSessionImpl)?.AutoFlushIfRequiredAsync(querySpaces, cancellationToken);
	if (autoFlushIfRequiredTask != null)
	{
		return await (autoFlushIfRequiredTask).ConfigureAwait(false) ?? false;
	}
	return (implementor as AbstractSessionImpl)?.AutoFlushIfRequiredAsync(querySpaces, cancellationToken) ?? false;
}

Compilation error:
error CS0019: Operator '??' cannot be applied to operands of type 'bool' and 'bool'

P.S. It's not something that I really need just noticed when tested some stuff

@maca88 maca88 added the bug label Dec 23, 2018
@maca88 maca88 closed this as completed in a55aac5 Dec 25, 2018
@maca88
Copy link
Owner

maca88 commented Dec 25, 2018

I've changed the generation logic for a Null-conditional operator when the return value is used afterwards.
The problem of generating an if statement in this case is that without knowing the end value type that will be used afterwards is not always possible to create a valid code.
Example:

public int? Test()
{
  var number = GetReader()?.ReadContentAsString().Length;
  // A lot of code that may use the number variable...
  return number > 5 ? number : 0;
}

The old behavior copied the whole code after the invocation statement, but as we can see below the code is invalid because of duplicate variable names:

// Old method - not working
public async Task<int?> TestAsync()
{
  var task = GetReader()?.ReadContentAsStringAsync();
  if (task != null)
  {
    var number = (await task).Length;
    // A lot of code that may use the number variable...
    return number > 5 ? number : 0;
  }
  var number = GetReader()?.ReadContentAsString().Length; // Error: number already declared
  // A lot of code that may use the number variable...
  return number > 5 ? number : 0;
}

The only solution that I found and implement to solve the above problem without knowing the type of the number variable is to use a ternary if:

// Current solution
public async Task<int?> TestAsync()
{
  var reader = GetReader();
  var number = (reader == null ? null : await reader.ReadContentAsStringAsync())?.Length;
  // A lot of code that may use the number variable...
  return number > 5 ? number : 0;
}

In your example the following code will be generated:

var autoFlushIfRequired0 = (implementor as AbstractSessionImpl);
return (autoFlushIfRequired0 == null ? (bool?) null : await (autoFlushIfRequired0.AutoFlushIfRequiredAsync(querySpaces, cancellationToken)).ConfigureAwait(false)) ?? false;

I know that it is not ideal, but due to the current limitations of the transformation process it is the only solution for now.
The ideal solution would be:

public async Task<int?> TestAsync()
{
  int? number = null;
  var task = GetReader()?.ReadContentAsStringAsync();
  if (task != null)
  {
    number = (await task)?.Length;
  }
  // A lot of code that may use the number variable...
  return number > 5 ? number : 0;
}

Also having multiple async invocation with a Null-conditional operator in the same statement is currently not supported even with the new logic. Example:

// Will not work if GetReader and ReadContentAsString have an async counterpart 
var number = instance?.GetReader()?.ReadContentAsString().Length;

@maca88 maca88 added this to the 0.13.2 milestone Feb 26, 2019
maca88 added a commit that referenced this issue Aug 25, 2019
…eturn value is used afterwards, fixes #117

# Conflicts:
#	Source/AsyncGenerator/Transformation/Internal/ReturnTaskFunctionRewriter.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants