Page MenuHomePhabricator

[LLD][COFF] Support /ignore:4099. Support /ignore with comma-separated arguments.

Authored by aganea on Jan 7 2019, 7:56 AM.




/ignore:4099 corresponds to "PDB 'filename' was not found with 'object/library' or at 'path'; linking object as if no debug info".

Without /ignore:4099, we can't enable /WX (treat warnings as errors).

Also, providing multiple arguments such as /ignore:4037,4049,4065,4098,4099,4217,4221 now works.

Diff Detail


Event Timeline

aganea created this revision.Jan 7 2019, 7:56 AM
This revision is now accepted and ready to land.Jan 7 2019, 10:29 AM
ruiu added a subscriber: ruiu.Jan 7 2019, 10:39 AM
ruiu added inline comments.
991 ↗(On Diff #180499)

What StringRef::split returns is not an iterator, so I think It is not a very good name. Since this is not a performance-critical path, maybe this is the simplest code I could think of:

SmallVector<StringRef, 1> Vec;
StringRef(Arg->getValue())->split(Vec, ',');
for (StringRef S : Vec) {
  if (S == "4037") {

You can find the same pattern in the code handling OPT_opt.

1304 ↗(On Diff #180499)

Flip the condition to return early.

1305 ↗(On Diff #180499)

Please avoid using auto unless its type is obvious from the right-hand side (for example, if the rhs is a dyn_cast, then auto is fine, but in this case the actual type is not obvious).

1306 ↗(On Diff #180499)

I don't know if adding [LNK4099] is the right thing. We have a lot of warnings and error messages in lld, but no one includes such error code. If we want to do this (I personally don't want though), we should do for everything, but this needs to be discussed first.

aganea marked an inline comment as done.Jan 7 2019, 10:47 AM
aganea added inline comments.
1306 ↗(On Diff #180499)

This is how other /ignore are handled (look for "[LNK4037]" and "[LNK4217]" in lld/trunk/COFF/Driver.cpp and lld/trunk/COFF/SymbolTable.cpp). This has been discussed in D44297.

aganea updated this revision to Diff 180543.Jan 7 2019, 12:01 PM
aganea marked 3 inline comments as done.

Updated as requested by @ruiu
Please let me know if that sounds better.

aganea updated this revision to Diff 180544.Jan 7 2019, 12:07 PM

Sorry I missed one line in PDB.cpp. Should be good now.

ruiu accepted this revision.Jan 7 2019, 1:20 PM


1 ↗(On Diff #180544)

This file is not a YAML file, so let's not use the .yaml file extension. Please name this ignore-many.test.

17 ↗(On Diff #180544)

Remove the extraneous last '\n'. (Your file ends with "\n\n", so you should remove the last '\n").

This revision was automatically updated to reflect the committed changes.