Fixed preprocessor bug which caused incorrect handling of the -D option #984
Conversation
| for (const auto& define : preprocessOptions.extraDefines) | ||
| context.add_macro_definition(define.identifier.data() + core::string("=") + define.definition.data()); | ||
| { | ||
| const std::string macroDefinition = define.identifier.data() + core::string("=") + define.definition.data(); |
There was a problem hiding this comment.
do we suppose define.definition being a null span ?
There was a problem hiding this comment.
define.definition can be an empty string but not a null span
| macroDefinitions.emplace_back(macroDefinitionBuffer.identifier, macroDefinitionBuffer.definition); | ||
| } | ||
|
|
||
| opt.preprocessorOptions.extraDefines = macroDefinitions; |
There was a problem hiding this comment.
ok but you forgot about preprocessOnly mode
| { | ||
| const std::string macroDefinition = define.identifier.data() + core::string("=") + define.definition.data(); | ||
| const bool isMacroAdded = context.add_macro_definition(macroDefinition); | ||
| assert(isMacroAdded); |
There was a problem hiding this comment.
a duplicate should not be asserted, in this case silent fail is better
you actually better just move .add_macro_definition into try block to let wave throw an exception which we handle nicely than assert (then we come back to silent fail + opportunity to catch stuff)
| const size_t equalPos = argumentTmp.find('='); | ||
| if (equalPos == std::string::npos) | ||
| { | ||
| identifier = argumentTmp; | ||
| definition = ""; |
There was a problem hiding this comment.
it changes semantic of -D compared to how DXC/Clang/GCC and friends handle it
lack of = means empty definition now, try adding -DNAME and then
#if NAME#if NAME == 1#if defined(NAME) && NAME#if NAME >= 2int x = NAME;
on -DNAME we should be defining 1 (-DNAME == -DNAME=1) in this case otherwise those above will fail
No description provided.