sammccall added a comment.
Thanks, test seems sensible.
I remain unconvinced that the umbrella stuff belongs here.
Maybe pull it out of this patch and send it separately if you want to discuss
further, or get more opinions?
================
Comment at: clang/lib/Lex/HeaderSearch.cpp:1962
+ } else {
+ // Prefer using `<UIKit/UIKit.h>` for non-`UIKit` main files.
+ //
----------------
dgoldman wrote:
> sammccall wrote:
> > This heuristic looks iffy to me, it's not just trying to help the caller
> > insert the right header, but tell them they want a different one for style
> > reasons.
> >
> > The first example framework in
> > https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/FrameworkAnatomy.html
> > doesn't include an umbrella header of this form.
> >
> > Moreover, non-framework libraries have umbrella headers too, this seems
> > like an orthogonal concern best caught in some other way.
> See
> https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPFrameworks/Tasks/IncludingFrameworks.html,
> it's common to use this umbrella header (especially for Apple SDKs) and some
> frameworks require it (as opposed to including the header directly), thus I
> think this default makes sense even if it isn't 100%.
>
> Any other ideas on how we can help here? I guess if we had FS access we could
> see if the umbrella header exists...
I get that it's often a good idea/mandatory to include the umbrella header (for
many libraries, not just frameworks), but it's not this function's job to
choose which file to include, it's to find the best spelling of the file that
was requested.
Similarly it doesn't check if the filename is "bits/shared_ptr.h" and suggest
`<shared_ptr>` instead.
> Any other ideas on how we can help here?
It depends what you're trying to achieve, maybe write a clang-tidy check
instead? Or if this is something the compiler itself needs to care about, write
a separate function to do the heuristic correction.
If you need the spelled, heuristically-corrected name, you can correct it first
and then call this code, rather than having the spelling code do heuristic
correction.
================
Comment at: clang/test/Modules/double-quotes.m:27
// CHECK: double-quoted include "A0.h" in framework header, expected
angle-bracketed instead
+// CHECK: <A/A0.h>
// CHECK: double-quoted include "B.h" in framework header, expected
angle-bracketed instead
----------------
it's hard to understand the context of how these headers are named, is there
any other text on the line you can include in the CHECK?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115183/new/
https://reviews.llvm.org/D115183
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
