RomanYankovsky / DelphiAST

Abstract syntax tree builder for Delphi
Mozilla Public License 2.0
271 stars 116 forks source link

Multiple spaces after IFDEF is causing defect #269

Open sglienke opened 5 years ago

sglienke commented 5 years ago

Following code is a simplified test case for this defect - originally found when trying to parse System.pas from Delphi 10.2

unit Unit1;

interface

implementation

procedure Foo;
{$IFDEF  CPUX86}
begin
end;
{$ENDIF}

initialization

end.

I traced it down to TmwBasePasLex.GetDirectiveParam returning the string with a leading space character in this case.

sglienke commented 5 years ago

I fixed this by changing the before mentioned method as follows:

-  if FBuffer.Buf[TempRun] = ' ' then Inc(TempRun);
+  while FBuffer.Buf[TempRun] = ' ' do Inc(TempRun);
+  while FBuffer.Buf[EndPos - 1] = ' ' do Dec(EndPos);

Only the first added line is in fact necessary to make System.pas being parsed again but I added the second one to see if that fixes any trailing spaces.

However there are more defects like this. For example following code still fails because of the implementation of TmwBasePasLex.EvaluateConditionalExpression.

unit Unit1;

interface

implementation

procedure Foo;
{$IF defined( CPUX86 )}
begin
end;
{$ENDIF}

initialization

end.

I think this will also fail because of tabs which is why I am not proposing some pull request for this because my fixes are just incomplete.

JBontes commented 5 years ago

Why not just do:

while CharInSet(FBuffer.Buf[TempRun], whitespace) do Inc(TempRun);
while CharInSet(FBuffer.Buf[EndPos - 1], whitespace) do Dec(EndPos);

Where whitespace is [#32,#9,#10,#13] and whatever else happens to qualify.
Not sure about unicode issues though.

sglienke commented 5 years ago

Why not just do:

while CharInSet(FBuffer.Buf[TempRun], whitespace) do Inc(TempRun);
while CharInSet(FBuffer.Buf[EndPos - 1], whitespace) do Dec(EndPos);

Where whitespace is [#32,#9,#10,#13] and whatever else happens to qualify. Not sure about unicode issues though.

Simple, because IFDEF cannot span multiple lines. I mentioned tabs because I noticed but there are also not handled in some other places where they should be valid so I would not address that one in the context of this particular issue.

bogdanpolak commented 2 years ago

@RomanYankovsky it look like Resolved. Maybe time to close the issue? :-)