[PATCH] D47157: Warning for framework headers using double quote includes

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a subscriber: arphaman.
bruno added inline comments.


================
Comment at: lib/Lex/HeaderSearch.cpp:753-754
+                  IncluderAndDir.second->getName()))
+            Diags.Report(IncludeLoc,
+                         diag::warn_quoted_include_in_framework_header)
+                << Filename;
----------------
dexonsmith wrote:
> bruno wrote:
> > aaron.ballman wrote:
> > > This seems like a good place for a fix-it to switch the include style. Is there a reason to not do that work for the user?
> > Like I explained above, we don't know which framework the header could be part of, so a fix-it could be misleading.
> Clang supports editor placeholders, which we use in some refactoring-style fix-its.  I think this would be spelled `<#framework-name#>`, or `#include <<#framework-name#>/Foo.h>`
My current understanding (after chatting with @arphaman) is that editor placeholders isn't a great fit here:

- For non IDE uses of this, it will just be weird to output something like `#include <<#framework-name#>/Foo.h>`. Even if we wanted to emit this only for IDE use, clang currently has no way to make that distinction (editor placeholder related compiler flags only make sense when actually making the special token sequence lexable, not when generating it)
- Fixits are (with some known exceptions) meant to be applied to code and subsequently allow compilation to succeed, this wouldn't be the case here.


Repository:
  rC Clang

https://reviews.llvm.org/D47157