Field names may clash with C keywords when compiling with C backend

[dlongnecke-cray]

Field names may clash with C keywords when compiling with C backend

When compiling codes with an aggregate containing a field name that is the same name as a C keyword, said name is not mangled. This causes the C compiler to emit errors when compiling the C code generated by Chapel.

record r {
  // This field is not mangled.
  var default: string;
}
proc test() {
  var r1 = new r('foo');
  writeln(r1);
}
test();

In the compiler function protectNameFromC, we currently do not rename fields:

  // Don't rename fields
  if (isVarSymbol(sym)) {
    if (isAggregateType(sym->defPoint->parentSymbol->type)) {
      return;
    }
  }

This issue proposes mangling aggregate fields that may clash with C keywords and asks about the potential consequences/pitfalls of doing so.

[dlongnecke-cray]

Mentioning @arezaii who ran into this issue today.

[mppf]

Do you think it'd be possible to only mangle names matching a keyword in a list? Mangling all of the field names seems like it would be annoying to deal with for other reasons (both in the compiler and when debugging etc).

[dlongnecke-cray]

Yes @mppf, I propose to only mangle field names that clash with C reserved words, and no others.

[bradcray]

I haven't done the full archeology to verify my sense of things, but I wouldn't be surprised if there was some semi-circular evolution of thought here (probably on my part) that left us exposed. I'm imagining something like:

• "let's mangle all reserved C identifiers when they appear in Chapel code!"
• "but wait, we could conflict with any non-reserved C identifiers as well, so let's mangle all Chapel identifiers and then we won't have to maintain that annoying list of reserved C identifiers!"
• "but wait, why are we mangling fields given that they can't conflict with a C identifier due to scoping (thinking only of the non-reserved case), let's tamp our mangling down to just things that can appear at file scope in the generated C code."

Specifically, note that we are still maintaining a list of reserved C symbols in compiler/codegen/reservedSymbolNames (which becomes compiler/codegen/reservedSymbolNames.h) and that it includes auto. And PR #14487 definitely suggests the kind of flawed thinking in bullet 3 above.

Also, a quick sanity check: Is it correct that we don't have to mangle reserved C identifiers in the field context when using the LLVM back-end? (since they're not parsed from source as with the C back-end). The title of this issue definitely suggests that, but I wanted to make sure.

[dlongnecke-cray]

Also, a quick sanity check: Is it correct that we don't have to mangle reserved C identifiers in the field context when using the LLVM back-end? (since they're not parsed from source as with the C back-end). The title of this issue definitely suggests that, but I wanted to make sure.

I don't think we have to worry about conflicts with keywords when generating LLVM instructions (though we do rename any symbols that might conflict with LLVM intrinsics, but that's sort of a different matter).

Evidence supporting this: The example code compiles under CHPL_LLVM=bundled.

And PR #14487 definitely suggests the kind of flawed thinking in bullet 3 above.

Thanks for pointing out the history! I went back about a year before I gave up looking.