bruno added inline comments.
================ Comment at: include/clang/Basic/DiagnosticLexKinds.td:713 +def warn_quoted_include_in_framework_header : Warning< + "double-quoted include \"%0\" in framework header, expected system style <angled> include" + >, InGroup<FrameworkHdrQuotedInclude>, DefaultIgnore; ---------------- dexonsmith wrote: > aaron.ballman wrote: > > bruno wrote: > > > aaron.ballman wrote: > > > > 80-col limit? > > > > > > > > Also, I'd probably drop "system style" and reword slightly to: > > > > > > > > `"double-quoted include \"%0\" in framework header, expected > > > > angle-bracketed include <%0> instead"` > > > Unfortunately this won't work because for framework style includes we use > > > the angled-bracketed with the framework name. For example, if one wants > > > to include `Foo.h` from `Foo.framework`, one should use `#include > > > <Foo/Foo.h>`, although on disk you actually have > > > `Foo.framework/Headers/Foo.h`. Framework header lookup does this magic > > > and other similar ones. > > > > > > Since we don't know which framework the quoted header could be part of, > > > it was not included in the warning (doing so would require extra header > > > searches - which could be expensive for this specific warning). However > > > it seems that I can do better to indicate that the framework name is > > > desired here, perhaps: > > > > > > `"double-quoted include \"%0\" in framework header, expected > > > angle-bracketed include <FrameworkName/%0> instead"` > > > > > > How does that sound to you? Other suggestions? > > Thank you for the explanation! > > > > I think your suggested text sounds good, though I do wonder how expensive > > is "expensive" in finding the intended header? Not only would it provide a > > better diagnostic, it would also let you use a fixit that doesn't use > > editor placeholders. > I'm also interested in just how expensive it would be, because I think users > will be frustrated that the compiler knows it's a framework include "so it > obviously knows which framework". > > I'd be fine if the fix-it came in a follow-up commit though (not sure how > Aaron feels). I haven't measured, but for each quoted include we would have to: - Start a fresh header search. - Look for `Foo.h` in all possible frameworks in the path (just on the Darwin macOS SDK path that would be around 140 frameworks). - If it's only found in once place, we are mostly safe to say we found a matching framework, otherwise we can't emit a reliable fixit. - Header maps and VFS might add extra level of searches (this is very common in Xcode based clang invocations). Repository: rC Clang https://reviews.llvm.org/D47157 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
