fix: clang_tidy - Add defines and local_defines attributes to commandline args #477
Conversation
…nd `local_defines` attributes
|
|
| @@ -270,6 +273,12 @@ def _get_compiler_args(ctx, compilation_context, srcs): | |||
| args.append("-D" + define) | |||
| for define in compilation_context.local_defines.to_list(): | |||
| args.append("-D" + define) | |||
| if hasattr(ctx.rule.attr, "defines"): | |||
There was a problem hiding this comment.
hmm, is it a bug that the compilation_context didn't already supply this local_define value? @jsharpe in case you have an opinion
There was a problem hiding this comment.
Certainly seems like it, but given its the behaviour of bazel releases then we've got to fix up for it; there shouldn't be too much harm (other than line length and the associated increase in memory use) in adding the defines to the commandline multiple times?
|
@molar are you able to click through the Contributor License Agreement? |
I am waiting for legal to look it over, not sure of the ETA. |
@alexeagle i have signed cla - is there any other issues you would like to be addressed |
The clang_tidy aspect did not read the
definesandlocal_definesattribute of the rule, causing clang_tidy to erroron parsing code that references these defines. This PR extends the clang_tidy.bzl with support for reading these attributes and adds them to the clang-tidy commandline.
I also added a guard against rules that do not have a
srcsattribute, but do return a CcInfo.Changes are visible to end-users: yes/no
Test plan
Extended the example with a
local_definesattribute and tried to lint the code without my changes, where i got theerror
after my change the error is gone and the normal fixes are proposed when running
bazel lint //src/cpp/....I only tested on ubuntu x86.