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

clang-format breaks code by splitting a string in a macro call. #17496

Closed
llvmbot opened this issue Sep 6, 2013 · 7 comments
Closed

clang-format breaks code by splitting a string in a macro call. #17496

llvmbot opened this issue Sep 6, 2013 · 7 comments
Labels
bugzilla Issues migrated from bugzilla clang-format

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 6, 2013

Bugzilla Link 17122
Resolution FIXED
Resolved on Sep 17, 2013 04:03
Version trunk
OS Windows NT
Reporter LLVM Bugzilla Contributor
CC @chandlerc,@zygoloid

Extended Description

Assume you have a macro defined as follows:
_T(x) L ## x

Where L"A string" makes wchar_t literal

Now when you have code like this:
_T("Some really long string that needs to be split into multiple lines")

clang-format will output:

_T("Some really long string that needs"
" to be split into multiple lines")

On visual studio 2010, this appears to be expanded as:
L"Some really long string that needs" " to be split into multiple lines"

and I get an error:
error C2308: concatenating mismatched strings

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Sep 6, 2013

This is an MSVC bug. Per [lex.string]p13, "If one string literal has no encoding-prefix, it is treated as a string literal of the same encoding-prefix as the other operand."

It's not clear to me how practical it would be for us to work around this bug. =(

@chandlerc
Copy link
Member

This is a really common Windows idiom, so I think we should try to support it. It doesn't seem too hard.

Wrap:
_T("really long string")

To:
_T("really")
_T("long")
_T("string")

And we should recognize it by finding a string literal inside of a '_T(...)' like thing.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 9, 2013

What exactly is a "_T(...)" like thing? If we are looking for exactly for the identifier _T, this could work. Otherwise, I think we would have to be absolutely sure that it is a macro defined as "L##x", because basically everything else will break with this change.

Seems like a big risk to work around a compiler bug. Alternatively, we could detect this pattern and not insert a break.

@chandlerc
Copy link
Member

What exactly is a "_T(...)" like thing? If we are looking for exactly for
the identifier _T, this could work. Otherwise, I think we would have to be
absolutely sure that it is a macro defined as "L##x", because basically
everything else will break with this change.

Yea, I was thinking to look for exactly '_T' preceding a parenthesis.

Seems like a big risk to work around a compiler bug. Alternatively, we could
detect this pattern and not insert a break.

My impression is that this is not dissimilar to the _(...) trick used in gettext in that it shows up in lots of places. That's why I thought it might be reasonable and worthwhile to do.... Fixing the compiler bug is both unlikely and slow.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 9, 2013

As I said, as long as we are looking for exactly "_T", I don't see problems. If not, I would prefer not introducing a line break (which would also "solve" this bug).

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 12, 2013

This is an MSVC bug. Per [lex.string]p13, "If one string literal has no
encoding-prefix, it is treated as a string literal of the same
encoding-prefix as the other operand."

It's not clear to me how practical it would be for us to work around this
bug. =(

This behavior conforms with the C++03 standard ([lex.string]p3):
"In translation phase 6 (2.1), adjacent narrow string literals are concatenated and adjacent wide string literals are concatenated. If a narrow string literal token is adjacent to a wide string literal token, the behavior is undefined."

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 17, 2013

AFAICT this is fixed by Alex's r190804.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang-format
Projects
None yet
Development

No branches or pull requests

2 participants