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

fix Issue 22812 - ImportC: C11 does not allow newlines between the start and end of a directive #13728

Merged
merged 3 commits into from
Mar 22, 2022

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Feb 27, 2022

Fix is identical to #13727, but fixes parsing in pragmaPack instead of poundLine.

@dlang-bot
Copy link
Contributor

dlang-bot commented Feb 27, 2022

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
22812 normal ImportC: C11 does not allow newlines between the start and end of a directive
22825 normal #line parsing doesn't follow the spec

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#13728"

src/dmd/lexer.d Outdated Show resolved Hide resolved
src/dmd/lexer.d Outdated Show resolved Hide resolved
src/dmd/lexer.d Outdated Show resolved Hide resolved
src/dmd/lexer.d Outdated Show resolved Hide resolved
src/dmd/lexer.d Outdated Show resolved Hide resolved
Copy link
Member

@WalterBright WalterBright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comments

src/dmd/lexer.d Outdated Show resolved Hide resolved
@ibuclaw
Copy link
Member Author

ibuclaw commented Mar 17, 2022

@WalterBright as you can see from all the errors, unsetting tokenizeNewlines at the point where a newline is seen doesn't work as-is.

==============================
Test 'fail_compilation/test21008.d' failed: 
expected:
----
fail_compilation/test21008.d(110): Error: function `test21008.C.after` circular reference to class `C`
fail_compilation/test21008.d(117): Error: need `this` for `toString` of type `string()`
fail_compilation/test21008.d(117): Error: need `this` for `toHash` of type `nothrow @trusted $?:32=uint|64=ulong$()`
fail_compilation/test21008.d(117): Error: function `object.Object.opCmp(Object o)` is not callable using argument types `()`
fail_compilation/test21008.d(117):        missing argument for parameter #1: `Object o`
fail_compilation/test21008.d(117): Error: function `object.Object.opEquals(Object o)` is not callable using argument types `()`
fail_compilation/test21008.d(117):        missing argument for parameter #1: `Object o`
fail_compilation/test21008.d(117): Error: `Monitor` has no effect
fail_compilation/test21008.d(117): Error: function `object.Object.factory(string classname)` is not callable using argument types `()`
fail_compilation/test21008.d(117):        missing argument for parameter #1: `string classname`
fail_compilation/test21008.d(105):        called from here: `handleMiddlewareAnnotation()`
fail_compilation/test21008.d(108): Error: class `test21008.C` no size because of forward reference
----
actual:
----
fail_compilation/test21008.d(99): Error: declaration expected, not `\n`
fail_compilation/test21008.d(106): Error: unmatched closing brace
----

@ibuclaw
Copy link
Member Author

ibuclaw commented Mar 18, 2022

Adapted poundLine() to call scan() to get each token in a #line directive.

The scope(exit) remains as it still is needed to catch the error case when parsing a special token sequence.

@ibuclaw ibuclaw force-pushed the issue22812 branch 3 times, most recently from 89a5460 to 9e9ed95 Compare March 18, 2022 16:46
@ibuclaw
Copy link
Member Author

ibuclaw commented Mar 18, 2022

scope(exit) has been removed.

Ping @WalterBright

1 similar comment
@ibuclaw
Copy link
Member Author

ibuclaw commented Mar 18, 2022

scope(exit) has been removed.

Ping @WalterBright

@dkorpel
Copy link
Contributor

dkorpel commented Mar 21, 2022

dwt ~master: building configuration "linux-gtk"...
base/src/java/nonstandard/UnsafeUtf.d-mixin-12(13,1): Error: undefined escape hex sequence \Ut

I don't see how this PR leads to that buildkite failure.

@ibuclaw
Copy link
Member Author

ibuclaw commented Mar 21, 2022

dwt ~master: building configuration "linux-gtk"...
base/src/java/nonstandard/UnsafeUtf.d-mixin-12(13,1): Error: undefined escape hex sequence \Ut

I don't see how this PR leads to that buildkite failure.

The spec needs changing: dlang/dlang.org#3257.

DWT has been fixed: d-widget-toolkit/dwt#107

@dlang-bot dlang-bot merged commit d682eac into dlang:master Mar 22, 2022
@ibuclaw ibuclaw deleted the issue22812 branch March 22, 2022 14:56
@ljmf00
Copy link
Member

ljmf00 commented Mar 22, 2022

This introduced trailing whitespace. Fix: #13863

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

Successfully merging this pull request may close these issues.

6 participants