Page MenuHomePhabricator

[clang-include-fixer] Skip .rc files when finding symbols

Authored by rnk on Nov 13 2019, 11:31 AM.



For some reason CMake includes entries for .rc files, but
find-all-symbols handles them improperly.

See PR43993

Diff Detail

Event Timeline

rnk created this revision.Nov 13 2019, 11:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 13 2019, 11:31 AM
sammccall accepted this revision.Nov 13 2019, 12:38 PM

This seems fine as a workaround, but will obviously only affect include-fixer. I'm wary of embedding it deeper without understanding the problem though.

Can you paste the *.rc entry from compile_commands.json?
I'm curious how CMake wants to send it to clang, and why it works with clang but not other tools.

This revision is now accepted and ready to land.Nov 13 2019, 12:38 PM
rnk added a comment.Nov 14 2019, 10:04 AM

This is a single .rc entry:

  "directory": "C:/src/llvm-project/build",
STDC_LIMIT_MACROS -DRC_FILE_VERSION=\\\"10.0.0git\\\" -DRC_INTERNAL_NAME=\\\"llvm-tblgen\\\" -DRC_PRODUCT_NAME=\\\"LLVM\\\" -DRC_PRODUCT_VERSION=\\\"10.0.0git\\\" -DRC_VERS
ION_FIELD_1=10 -DRC_VERSION_FIELD_2=0 -DRC_VERSION_FIELD_3=0 -DRC_VERSION_FIELD_4=0 -IC:\\src\\llvm-project\\build\\utils\\TableGen -IC:\\src\\llvm-project\\llvm\\utils\\Ta
bleGen -IC:\\src\\llvm-project\\build\\include -IC:\\src\\llvm-project\\llvm\\include  /DWIN32   -UNDEBUG /nologo /foutils\\TableGen\\CMakeFiles\\llvm-tblgen.dir\\__\\__\\r
esources\\windows_version_resource.rc.res C:\\src\\llvm-project\\llvm\\resources\\windows_version_resource.rc",
  "file": "C:/src/llvm-project/llvm/resources/windows_version_resource.rc"

There is one for every binary. I think .rc files can include headers to get things like struct sizes and macros. My understanding is that it's an old fork of the Visual C compiler, so it is in some sense a "compilation". I suppose if you wanted to write a global renaming tool for a macro, you'd need to know about uses in .rc files.

This revision was automatically updated to reflect the committed changes.
kimgr added a subscriber: kimgr.Nov 16 2019, 2:52 AM

I wonder if it would be better to look at the compile-command than the source file?

It's not unthinkable that someone would use another extension for an rc file, or use a .rc extension for a C or C++ source file. But if the compiler is rc.exe, the source file must be in rc format and should be ignored, right?

rnk added a comment.Nov 17 2019, 1:31 PM

We already have llvm-rc.exe which one could plausibly use to compile rc files, and I didn't like binary.lower().endswith('rc.exe') as a test.

Oh, I see. I thought rc.exe was the one and only implementation. Fair enough, this is probably the lesser of two evils :)