This is an archive of the discontinued LLVM Phabricator instance.

Fix a bug that warnings generated with -M or -MM flags

Authored by yamaguchi on Apr 21 2017, 12:52 AM.



This is a patch for bug [1]

Warnings should not be emitted with -M and -MM flags, because this mode is only used for generate MakeFiles.


Diff Detail

Event Timeline

yamaguchi created this revision.Apr 21 2017, 12:52 AM
yamaguchi added reviewers: ruiu, teemperor, v.g.vassilev.
v.g.vassilev edited edge metadata.Apr 21 2017, 1:10 PM

Could you add a test case?

yamaguchi updated this revision to Diff 96273.Apr 21 2017, 10:20 PM

Add testcase.

LGTM modulo the comments.



yamaguchi updated this revision to Diff 96283.Apr 22 2017, 6:07 AM

Fixed typo.

yamaguchi marked an inline comment as done.Apr 22 2017, 6:08 AM
teemperor requested changes to this revision.Apr 23 2017, 2:47 PM

This also disables warnings for -MD and -MMD which shouldn't happen as we're actually compiling code here and are probably compiling a project. E.g. for people that use this mode to create dependency files while initially building (to figure out when to rebuild each object), this will effectively force -w to the whole build process which is not good.

I would just manually check for -M and -MM where we only run the preprocessor and where adding -w is safe. Also please add another test that we actually get the warnings for -MD and another one for -MMD.

Otherwise this look good.

This revision now requires changes to proceed.Apr 23 2017, 2:47 PM
yamaguchi added a comment.EditedApr 24 2017, 1:46 AM

Do you think it is not a good idea to change condition if -MD or -MMD exists?
for example,
if ( !A->getOption().matches(options::OPT_MD) && !A->getOption().matches(options::OPT_MMD)) CmdArgs.push_back("-w");

I think you will see the issue if you add a negative test case, too. I.e. as Raphael said in -MD and -MMD mode we should be able to see warnings.

yamaguchi updated this revision to Diff 97013.Apr 27 2017, 4:08 PM
yamaguchi edited edge metadata.

show warnings with -M and -MD

yamaguchi updated this revision to Diff 97019.Apr 27 2017, 4:24 PM

Add testcase

@teemperor, is this ok with you?

teemperor accepted this revision.Jun 14 2017, 2:11 AM

LGTM, good job! (Sorry for the delay, I think I got interrupted here by the GSoC start...)

This revision is now accepted and ready to land.Jun 14 2017, 2:11 AM
yamaguchi closed this revision.Jun 16 2017, 9:04 AM

Landed in r305561.