Page MenuHomePhabricator

[Preprocessor] For missing file in framework add note about framework location.
ClosedPublic

Authored by vsapsai on Jan 10 2019, 2:38 PM.

Details

Summary

When a framework with the same name is available at multiple framework
search paths, we use the first matching location. If a framework at this
location doesn't have all the headers, it can be confusing for
developers because they see only an error 'Foo/Foo.h' file not found,
can find the complete framework with required header, and don't know the
incomplete framework was used instead.

Add a note explaining a framework without required header was found.
Also mention framework directory path to make it easier to find the
incomplete framework.

rdar://problem/39246514

Diff Detail

Repository
rC Clang

Event Timeline

vsapsai created this revision.Jan 10 2019, 2:38 PM
vsapsai marked an inline comment as done.Jan 10 2019, 2:40 PM
vsapsai added inline comments.
clang/lib/Lex/Preprocessor.cpp
573–574 ↗(On Diff #181161)

I didn't enhance the diagnostics in this place for -pch-through-header case because it adds extra code and complexity with little benefit. The reason I think there'll be little benefit is because -pch-through-header is for MSVC-style precompiled headers while frameworks are used mostly on Darwin.

jkorous accepted this revision.Jan 24 2019, 12:24 PM

LGTM with nits about doxygen annotations.

clang/include/clang/Lex/DirectoryLookup.h
175 ↗(On Diff #181161)

We might make this easier to understand by explaining where/how the '.framework' directory is specified.

clang/include/clang/Lex/HeaderSearch.h
396 ↗(On Diff #181161)

We might try to explain what exactly is meant by corresponding .framework directory.

This revision is now accepted and ready to land.Jan 24 2019, 12:24 PM
vsapsai updated this revision to Diff 183678.Jan 25 2019, 6:33 PM
  • Tweak doxygen comment to express IsFrameworkFound in terms of IsFrameworkFound returned by DirectoryLookup.
vsapsai marked 2 inline comments as done.Jan 25 2019, 6:38 PM
vsapsai added inline comments.
clang/include/clang/Lex/DirectoryLookup.h
175 ↗(On Diff #181161)

This is a method in DirectoryLookup which has a method isFramework and constructor parameter isFramework. I think at this point users of DirectoryLookup should be aware of framework-style header lookup and this is not the right place to describe it.

clang/include/clang/Lex/HeaderSearch.h
396 ↗(On Diff #181161)

Tweaked the comment.

vsapsai updated this revision to Diff 184866.Feb 1 2019, 4:20 PM
  • Improve the diagnostic message wording.

Rebased the patch, so the diff between 2 revisions can be noisy.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2019, 2:34 PM