Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Functions are formatted even when passed to a macro which stringifies it #17951

Closed Quuxplusone closed 10 years ago

Quuxplusone commented 10 years ago
Bugzilla Link PR17952
Status RESOLVED WONTFIX
Importance P normal
Reported by Marcel Krems (marcel.krems@web.de)
Reported on 2013-11-15 20:21:49 -0800
Last modified on 2014-08-05 08:33:19 -0700
Version trunk
Hardware PC Linux
CC djasper@google.com, klimek@google.com, ssen@apple.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
#define MAKE_STRING(str) #str
MAKE_STRING(function(ParamType1*,ParamType2));

is changed to
MAKE_STRING(function(ParamType1 *, ParamType2));

The output should be unchanged, because it is used as a string and changing it
could change the behaviour of the program.

This is usually used in Qt's Signal & Slot system:
connect(sender, SIGNAL(destroyed(QObject*)), this,
SLOT(objectDestroyed(Qbject*)));
Quuxplusone commented 10 years ago
Can you expand on why changing the whitespace here is a problem for the Qt case?
(note that clang-format does not know about macro definitions, it only lexes)
Quuxplusone commented 10 years ago
From the Qt documentation:
"Qt uses normalized signatures to decide whether two given signals and slots
are compatible. Normalization reduces whitespace to a minimum, moves 'const' to
the front where appropriate, removes 'const' from value types and replaces
const references with values."

The connects will still work with any combination of whitespace characters but
since your signature doesn't have to be normalized you save some time.

For further clarification: http://marcmutz.wordpress.com/effective-qt/prefer-to-
use-normalised-signalslot-signatures/
Quuxplusone commented 10 years ago

Thx for the explanation. One question we already had was whether we'd want to make it possible to configure a few "stringify macro identifiers" (we have similar problems in the VS headers). I think that the use case for this is becoming more and more compelling. Thanks for reporting!

Quuxplusone commented 10 years ago

I've also run into a similar case, where after macro expansion, the string is used as a label in inline assembly. Adding whitespace caused the label name to have whitespace, which caused a compile failure. Arguably, clang-format should not be used for non-C code, but inline assembly is kind of special.

Quuxplusone commented 10 years ago

I think the solution here is to disable clang-format for certain regions of code (llvm.org/PR20463). With macros, it is otherwise always possible to come up with something that clang-format doesn't understand and thus breaks.