Page MenuHomePhabricator

Simplify error reporting in YAMLParser
Needs ReviewPublic

Authored by chfast on Jul 21 2016, 3:02 AM.

Details

Summary

This change removes Location param in setError() functions. It was never used.

Alternative to https://reviews.llvm.org/D22565

Diff Detail

Event Timeline

chfast updated this revision to Diff 64847.Jul 21 2016, 3:02 AM
chfast retitled this revision from to Simplify error reporting in YAMLParser.
chfast updated this object.
chfast added reviewers: arphaman, Bigcheese.
chfast added a subscriber: llvm-commits.
Bigcheese edited edge metadata.Jul 22 2016, 4:20 PM

There are some cases where it looks like the argument isn't the same as Current. Is this actually the case?

lib/Support/YAMLParser.cpp
998–999

This one appears to change behavior with this patch. I don't believe this will ever be the same token as Current.

chfast added inline comments.Jul 23 2016, 12:29 PM
lib/Support/YAMLParser.cpp
998–999

You are partly right. The information provided by the second argument is never used because the param in setError() function is unused. This is the true origin of this patch.

I've changes that in https://reviews.llvm.org/D22565. But apparently nobody cares about this detail in error reports as this has been unnoticed for years. So this patch keeps status quo and simplifies the code.

As you are doing something with YAML, can you take a look here?