This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Pass a TextDiagnosticPrinter when we can not create tempory file.
Needs RevisionPublic

Authored by Et7f3 on Jul 4 2021, 10:27 AM.

Details

Summary

Reproduction:
$ mkdir folder
$ echo ' ' > folder/file.c
$ sudo chown -R root folder
$ clang-format -i root/file.c

Before the clang-format abort now clang-format explain why it fail.

Ask: They are other DiagnosticConsumer without client should we update the other ?
Also we don't know how to create test that play with user permission.

Co-authored-by: "tartine8" <pbpcf8@gmail.com>

Diff Detail

Event Timeline

Et7f3 created this revision.Jul 4 2021, 10:27 AM
Et7f3 requested review of this revision.Jul 4 2021, 10:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2021, 10:27 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Et7f3 updated this revision to Diff 356391.Jul 4 2021, 11:52 AM
Et7f3 edited the summary of this revision. (Show Details)

Add a reproduction case in commit message.

xgupta added subscribers: MyDeveloperDay, xgupta.

Thanks for the patch @Et7f3! I tested it locally and can reproduce it with cat folder/file.c
int main()
{
return 0;
}
And indeed the patch also solved the crash issue and emit a correct error message but I am not sure about its implementation, I think @MyDeveloperDay can best review this patch.

There is a nit in the patch summary, clang-format -i root/file.c should be clang-format -i folder/file.c.

MyDeveloperDay added a project: Restricted Project.
MyDeveloperDay requested changes to this revision.Jul 5 2021, 8:19 AM

Including clangFrontend has been discourage many times before, we don't want to add that to the list of dependencies please see D90121: clang-format: Add a consumer to diagnostics engine

This revision now requires changes to proceed.Jul 5 2021, 8:19 AM
Et7f3 added a comment.Jul 6 2021, 8:16 AM

Mb I just looked in driver how to setup diagnostics (which included frontend).

Ok so I see 2 options:

  • move TextDiagnosticsPrinter in clangBasic
  • Use SMDiagnostic like clang/tools/clang-format/ClangFormat.cpp:324 but it seem to need a SMLoc so not really adapted.

Yeah, I made the almost identical change but got some push back on it extending the dependencies (I think it also meant that the clang-format target needed alot more source to be compiled)

Dinking around in clangBasic is probably outside my pay grade ;-)

This is no longer needed, please abanson