Page MenuHomePhabricator

[clang-format] Suppress diagnostics on second parse
ClosedPublic

Authored by HazardyKnusperkeks on Feb 16 2021, 1:19 AM.

Details

Summary

This is the follow up to D93844.

Suppress the diagnostics when applying the child configurations, since they were printed on the first parse.

Diff Detail

Event Timeline

HazardyKnusperkeks requested review of this revision.Feb 16 2021, 1:19 AM
HazardyKnusperkeks created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2021, 1:19 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
clang/include/clang/Format/Format.h
3344–3348

On my code I would do that change too.

This way we would get all the changes to the signature without manual work.

I did have another plan for a different way to go about this, not sure if its really any better though.
It involved building a vector of the edits the config would apply to its parent. Then after the parent is parsed just applying those edits.
It results in only 1 pass over the config file however it's a little more involved to get it to work.

clang/include/clang/Format/Format.h
3340

I know this is sufficient for the purpose of this patch, however it doesn't make sense to accept take the handler parameter without also taking a void* for its context.

I did have another plan for a different way to go about this, not sure if its really any better though.
It involved building a vector of the edits the config would apply to its parent. Then after the parent is parsed just applying those edits.
It results in only 1 pass over the config file however it's a little more involved to get it to work.

But then I need to rewrite the complete parsing process? Because in a normal style I can not parse, because it is not apparent which entries are changed because of the config, or do I oversee something?

As a follow up it may be wise to pass a diag handler to parseConfiguration as when we parse it a second time, we probably want to disregard any warnings (like unknown key) detected as they will have been printed on the first pass.

And this is what you proposed. :)

clang/include/clang/Format/Format.h
3340

Will do.

HazardyKnusperkeks marked an inline comment as done.

Added DiagHandlerCtxt.

I did have another plan for a different way to go about this, not sure if its really any better though.
It involved building a vector of the edits the config would apply to its parent. Then after the parent is parsed just applying those edits.
It results in only 1 pass over the config file however it's a little more involved to get it to work.

But then I need to rewrite the complete parsing process? Because in a normal style I can not parse, because it is not apparent which entries are changed because of the config, or do I oversee something?

As a follow up it may be wise to pass a diag handler to parseConfiguration as when we parse it a second time, we probably want to disregard any warnings (like unknown key) detected as they will have been printed on the first pass.

And this is what you proposed. :)

The diaghandler is a perfectly good solution, I was just spit balling ideas. But it is possible to store changes, though not much is gained for this use case.

LGTM, but see what @MyDeveloperDay has to say.

This revision is now accepted and ready to land.Mar 4 2021, 8:42 AM