dexonsmith added inline comments.
================
Comment at: clang/test/Preprocessor/header-search-user-entries.c:9-11
+// CHECK: remark: user-provided search path used:
'{{.*}}/header-search-user-entries/a'
+// CHECK: remark: user-provided search path used:
'{{.*}}/header-search-user-entries/a_next'
+// CHECK: remark: user-provided search path used:
'{{.*}}/header-search-user-entries/d'
----------------
jansvoboda11 wrote:
> jansvoboda11 wrote:
> > jansvoboda11 wrote:
> > > dexonsmith wrote:
> > > > If `-verify` doesn't work (hopefully it does), I think you'll need to
> > > > add some `CHECK-NOT` / `CHECK-NEXT` / etc. or else you're not testing
> > > > for the absence of other diagnostics.
> > > >
> > > > Please also test:
> > > > - Framework search path (used vs. not used)
> > > > - System search path (used vs. not used)
> > > > - One search path described relative to `-isysroot` (used vs. not used)
> > > > - Header map (matched and used vs. matched but not used vs. not matched
> > > > -- for the middle case, I'm not sure what result we actually want)
> > > > - A path only used via `#if __has_include()` (when it's not followed by
> > > > an `#include` or `#import`)
> > > > - A path only used via ObjC's `#import`
> > > >
> > > > If one of them doesn't work and it's complex enough to separate into a
> > > > follow up, I think that'd be fine, but please find somewhere to leave a
> > > > FIXME.
> > > Good point, I'll work on improving the coverage.
> > I tried using `-verify` together with `expected-remark-re` (to match
> > **relative** paths) and `@*` (to match any/no line number), but those don't
> > seem to work together.
> >
> > This expectation:
> > ```
> > // expected-remark-re @* {{search path used:
> > '{{.*}}/search-path-usage/FwA'}}
> > ```
> > produces:
> > ```
> > error: 'error' diagnostics seen but not expected:
> > File
> > /Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/search-path-usage-headers.m
> > Line 30: cannot find start ('{{') of expected regex
> > error: 'remark' diagnostics seen but not expected:
> > (frontend): search path used:
> > '/Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/Inputs/search-path-usage/FwA'
> > 2 errors generated.
> > ```
> >
> > If I drop `@*`, stuff doesn't work (presumably because of line numbers):
> > ```
> > error: 'remark' diagnostics expected but not seen:
> > File
> > /Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/search-path-usage-headers.m
> > Line 30: search path used: '{{.*}}/search-path-usage/FwA'
> > error: 'remark' diagnostics seen but not expected:
> > (frontend): search path used:
> > '/Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/Inputs/search-path-usage/FwA'
> > 2 errors generated.
> > ```
> >
> > Let me know if I'm doing something wrong. In the meantime, I made the
> > FileCheck checks more thorough as you suggested.
> > - Header map (matched and used vs. matched but not used vs. not matched --
> > for the middle case, I'm not sure what result we actually want)
>
> Can you clarify what you mean by "matched" and "used" exactly?
Oh, I hadn't realized the diagnostic doesn't have a source location. Is there a
reason it has no location right now?
Seems like pointing at the first `#include` that triggers it would be ideal,
showing the include stack if there is one.
================
Comment at: clang/test/Preprocessor/header-search-user-entries.c:9-11
+// CHECK: remark: user-provided search path used:
'{{.*}}/header-search-user-entries/a'
+// CHECK: remark: user-provided search path used:
'{{.*}}/header-search-user-entries/a_next'
+// CHECK: remark: user-provided search path used:
'{{.*}}/header-search-user-entries/d'
----------------
dexonsmith wrote:
> jansvoboda11 wrote:
> > jansvoboda11 wrote:
> > > jansvoboda11 wrote:
> > > > dexonsmith wrote:
> > > > > If `-verify` doesn't work (hopefully it does), I think you'll need to
> > > > > add some `CHECK-NOT` / `CHECK-NEXT` / etc. or else you're not testing
> > > > > for the absence of other diagnostics.
> > > > >
> > > > > Please also test:
> > > > > - Framework search path (used vs. not used)
> > > > > - System search path (used vs. not used)
> > > > > - One search path described relative to `-isysroot` (used vs. not
> > > > > used)
> > > > > - Header map (matched and used vs. matched but not used vs. not
> > > > > matched -- for the middle case, I'm not sure what result we actually
> > > > > want)
> > > > > - A path only used via `#if __has_include()` (when it's not followed
> > > > > by an `#include` or `#import`)
> > > > > - A path only used via ObjC's `#import`
> > > > >
> > > > > If one of them doesn't work and it's complex enough to separate into
> > > > > a follow up, I think that'd be fine, but please find somewhere to
> > > > > leave a FIXME.
> > > > Good point, I'll work on improving the coverage.
> > > I tried using `-verify` together with `expected-remark-re` (to match
> > > **relative** paths) and `@*` (to match any/no line number), but those
> > > don't seem to work together.
> > >
> > > This expectation:
> > > ```
> > > // expected-remark-re @* {{search path used:
> > > '{{.*}}/search-path-usage/FwA'}}
> > > ```
> > > produces:
> > > ```
> > > error: 'error' diagnostics seen but not expected:
> > > File
> > > /Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/search-path-usage-headers.m
> > > Line 30: cannot find start ('{{') of expected regex
> > > error: 'remark' diagnostics seen but not expected:
> > > (frontend): search path used:
> > > '/Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/Inputs/search-path-usage/FwA'
> > > 2 errors generated.
> > > ```
> > >
> > > If I drop `@*`, stuff doesn't work (presumably because of line numbers):
> > > ```
> > > error: 'remark' diagnostics expected but not seen:
> > > File
> > > /Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/search-path-usage-headers.m
> > > Line 30: search path used: '{{.*}}/search-path-usage/FwA'
> > > error: 'remark' diagnostics seen but not expected:
> > > (frontend): search path used:
> > > '/Users/Jan/Code/upstream-llvm/clang/test/Preprocessor/Inputs/search-path-usage/FwA'
> > > 2 errors generated.
> > > ```
> > >
> > > Let me know if I'm doing something wrong. In the meantime, I made the
> > > FileCheck checks more thorough as you suggested.
> > > - Header map (matched and used vs. matched but not used vs. not matched
> > > -- for the middle case, I'm not sure what result we actually want)
> >
> > Can you clarify what you mean by "matched" and "used" exactly?
> Oh, I hadn't realized the diagnostic doesn't have a source location. Is there
> a reason it has no location right now?
>
> Seems like pointing at the first `#include` that triggers it would be ideal,
> showing the include stack if there is one.
> > - Header map (matched and used vs. matched but not used vs. not matched
>
> Can you clarify what you mean by "matched" and "used" exactly?
Assume "hmap" is a header map with the edge `a.h -> b.h`.
- "Matched" means the header map is queried with "a.h" (or "A.H", or whatever).
- "Used" means "b.h" is subsequently found on disk.
I think the way header maps work, a "matched but not used" would typically
result in a compile error... since once the path is converted to "b.h" there's
no going back. But here's a scenario would it wouldn't be an error:
```
lang=c++
#if __has_include("a.h")
#include "a.h"
#endif
```
The `__has_include("a.h")` would match in the header map, converting to
`"b.h"`, but there's no error when `"b.h"` isn't found on disk.
> > for the middle case, I'm not sure what result we actually want
Thinking about it more, I think we need it to count as a use of the header map
whenever there's a match (regardless of what happens downstream after the path
replacement), because it can affect the result.
With this setup:
```
% find . -type f
include/a.h
include/c.h
t.c
hmap
% clang -Ihmap -Iinclude -E t.c
```
(same header map, `a.h -> b.h`)
Here's an example of the "middle" case, matched but not used, where by my logic
above the header map needs a remark:
```
% cat t.c
#include "a.h"
```
... because removing the `-Ihmap` changes the result of the compilation.
Here's the third case, not matched, where the remark shouldn't trigger since
`-Ihmap` does not affect the compilation:
```
% cat t.c
#include "c.h"
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102923/new/
https://reviews.llvm.org/D102923
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
