Skip to content

Add space between ( and the binary in LIT tests#746

Open
Ris-Bali wants to merge 1 commit intoqualcomm:mainfrom
Ris-Bali:windows_trailing_character
Open

Add space between ( and the binary in LIT tests#746
Ris-Bali wants to merge 1 commit intoqualcomm:mainfrom
Ris-Bali:windows_trailing_character

Conversation

@Ris-Bali
Copy link
Contributor

@Ris-Bali Ris-Bali commented Jan 27, 2026

This patch adds space between the ( and the binary in some commands in LIT tests. These have been causing failures in specific windows environments.

The error occuring is

'(i:/bld/llvm/bin/not.exe': command not found

### because it crashes only in some code paths when doing LTO.

RUN: %clang %clangopts -c %p/Inputs/1.c -O2 -ffunction-sections -flto -o %t1.o
RUN: (%not %link %linkopts --save-temps %t1.o -o %t1.out 2>&1 || true) | %filecheck %s
Copy link
Member

Choose a reason for hiding this comment

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

The braces does not seem stray / without-any-purpose. For the windows issue, would adding a space between the brace ( and the lit substitution variable %not be enough?

Copy link
Contributor

@quic-seaswara quic-seaswara Jan 27, 2026

Choose a reason for hiding this comment

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

Remove || true. I dont think its needed.

@Ris-Bali Ris-Bali force-pushed the windows_trailing_character branch from 5b1220b to 840effc Compare January 27, 2026 09:24
@Ris-Bali Ris-Bali changed the title Remove stray () from eld tests Add space between ( and the binary in LIT tests Jan 27, 2026
@Ris-Bali Ris-Bali marked this pull request as draft January 27, 2026 11:43
@Ris-Bali Ris-Bali force-pushed the windows_trailing_character branch from 840effc to 50a91e8 Compare January 30, 2026 08:47
This patch adds a trailing space between the `(` and the binary
in commands for some tests.

Signed-off-by: Rishabh Bali <rbali@qti.qualcomm.com>
@Ris-Bali Ris-Bali force-pushed the windows_trailing_character branch from 50a91e8 to 4463b6d Compare January 30, 2026 08:49
@Ris-Bali Ris-Bali marked this pull request as ready for review January 30, 2026 17:16
@quic-akaryaki
Copy link
Contributor

Please add a reference to a failing build and copy the error here.

Copy link
Contributor

@quic-akaryaki quic-akaryaki left a comment

Choose a reason for hiding this comment

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

Please describe the error that happens on Windows.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants