Skip to content

Expand conditional code inlining capabilities #207

Description

@sglienke

I was not sure if I should add this to the already closed issue so I decided to create a new issue.

The existing behavior still creates an unreadable mess - the following code:

  Nullable<T> = {$IFDEF NULLABLE_PACKED}packed {$ENDIF}record
  private
    fValue: T;
    fHasValue: {$IFNDEF NULLABLE_CMR}string{$ELSE}Boolean{$ENDIF};

gets turned into

  Nullable<T> =
{$IFDEF NULLABLE_PACKED}
    packed
{$ENDIF}
      record
  private
    fValue: T;
    fHasValue:
{$IFNDEF NULLABLE_CMR}
      string
{$ELSE}
      Boolean
{$ENDIF}
      ;

And another one:

    class operator Implicit(const {$IFDEF SUPPORTS_CONSTREF}[ref]{$ENDIF}value: Lazy<T>): ILazy<T>;

turns into:

    class operator Implicit(
      const
{$IFDEF SUPPORTS_CONSTREF}
        [ref]
{$ENDIF}
        value: Lazy<T>
    ): ILazy<T>;

and I also found this which goes completely bonkers:

function IsNullable(typeInfo: PTypeInfo): Boolean;
const
  PrefixString = 'Nullable<';    // DO NOT LOCALIZE
begin
  Result := Assigned(typeInfo) and (typeInfo.Kind in [tkRecord{$IF Declared(tkMRecord)}, tkMRecord{$IFEND}])
    and StartsText(PrefixString, typeInfo.TypeName);
end;

turns into:

function IsNullable(typeInfo: PTypeInfo): Boolean;
const
  PrefixString = 'Nullable<'; // DO NOT LOCALIZE
begin
  Result :=
    Assigned(typeInfo)
      and (typeInfo.Kind
        in [
          tkRecord
{$IF Declared(tkMRecord)}
          ,
          tkMRecord
{$IFEND}
        ])
      and StartsText(PrefixString, typeInfo.TypeName);
end;

This last one is probably also related to what I reported in #205. Even though the overzealous line breaking is bad enough IMO it makes it look worse because it keeps indenting. While I admit that the original does not have the best formatting for readability the formatted one does no better IMHO.

This would already improve it a bit I guess:

function IsNullable(typeInfo: PTypeInfo): Boolean;
const
  PrefixString = 'Nullable<';    // DO NOT LOCALIZE
begin
  Result := Assigned(typeInfo) 
    and (typeInfo.Kind in [tkRecord{$IF Declared(tkMRecord)}, tkMRecord{$IFEND}])
    and StartsText(PrefixString, typeInfo.TypeName);
end;

The interesting thing here is that it does not only wrap around the conditionals but also more lines than necessary. If you remove the conditionals around the tkMRecord it turns this into a 2 liner.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions