[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[Arthur Eubanks via cfe-commits]

aeubanks wrote:

ah it's because we something like

```
[cfi-unrelated-cast|cfi-derived-cast]

src:*third_party/vulkan_memory_allocator/include/vk_mem_alloc.h
```

it seems like the new system doesn't match 
`[cfi-unrelated-cast|cfi-derived-cast]`

https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[Arthur Eubanks via cfe-commits]

aeubanks wrote:

the file name is `vk_mem_alloc.h` so that shouldn't be the issue

https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay closed 
https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/74809

>From 4aba7a65213971887db7432911e7743365f4f417 Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Thu, 7 Dec 2023 21:17:24 -0800
Subject: [PATCH 1/2] [SpecialCaseList] Use glob by default

https://reviews.llvm.org/D154014 addes glob support and enables it when
`#!special-case-list-v2` is the first line. This patch makes the glob
support the default (faster than regex after
https://reviews.llvm.org/D156046) and switches to the deprecated regex
support if `#!special-case-list-v1` is the first line.

I have surveyed many ignore lists. All ignore lists I find only use
basic `*` `.` and don't use regex metacharacters such as `(` and `)`.
(As neither `src:` nor `fun:` benefits from using regex.)
They are unaffected by the transition (with a caution that regex
`src:x/a.pb.*` matches `x/axpbx` but glob `src:x/a.pb.*` doesn't).

There is no deprecating warning. If a user finds
`#!special-case-list-v1`, they shall read that the old syntax is
deprecated.

Link: 
https://discourse.llvm.org/t/use-glob-instead-of-regex-for-specialcaselists/71666
---
 clang/docs/SanitizerSpecialCaseList.rst| 18 +++---
 llvm/lib/Support/SpecialCaseList.cpp   | 11 +--
 llvm/unittests/Support/SpecialCaseListTest.cpp |  8 
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/clang/docs/SanitizerSpecialCaseList.rst 
b/clang/docs/SanitizerSpecialCaseList.rst
index ab39276b043957..c7fb0fa3f8a828 100644
--- a/clang/docs/SanitizerSpecialCaseList.rst
+++ b/clang/docs/SanitizerSpecialCaseList.rst
@@ -56,13 +56,18 @@ and lines starting with "#" are ignored.
 
 .. note::
 
-  In `D154014 `_ we transitioned to using 
globs instead
-  of regexes to match patterns in special case lists. Since this was a
-  breaking change, we will temporarily support the original behavior using
-  regexes. If ``#!special-case-list-v2`` is the first line of the file, then
-  we will use the new behavior using globs. For more details, see
-  `this discourse post 
`_.
+  Prior to Clang 18, section names and entries described below use a variant of
+  regex where ``*`` is translated to ``.*``. Clang 18 (`D154014
+  `) switches to glob and plans to remove
+  regex support in Clang 19.
 
+  For Clang 18, regex is supported if ``#!special-case-list-v1`` is the first
+  line of the file.
+
+  Many special case lists use ``.`` to indicate the literal character and do
+  not use regex metacharacters such as ``(``, ``)``. They are unaffected by the
+  regex to glob transition. For more details, see `this discourse post
+  
`_.
 
 Section names are globs written in square brackets that denote
 which sanitizer the following entries apply to. For example, ``[address]``
@@ -80,7 +85,6 @@ tool-specific docs.
 
 .. code-block:: bash
 
-#!special-case-list-v2
 # The line above is explained in the note above
 # Lines starting with # are ignored.
 # Turn off checks for the source file
diff --git a/llvm/lib/Support/SpecialCaseList.cpp 
b/llvm/lib/Support/SpecialCaseList.cpp
index ac8877cca8bc66..7a23421eaeb89d 100644
--- a/llvm/lib/Support/SpecialCaseList.cpp
+++ b/llvm/lib/Support/SpecialCaseList.cpp
@@ -150,13 +150,12 @@ bool SpecialCaseList::parse(const MemoryBuffer *MB, 
std::string ) {
 return false;
   }
 
-  // In https://reviews.llvm.org/D154014 we transitioned to using globs instead
-  // of regexes to match patterns in special case lists. Since this was a
-  // breaking change, we will temporarily support the original behavior using
-  // regexes. If "#!special-case-list-v2" is the first line of the file, then
-  // we will use the new behavior using globs. For more details, see
+  // In https://reviews.llvm.org/D154014 we added glob support and planned to
+  // remove regex support in patterns. We temporarily support the original
+  // behavior using regexes if "#!special-case-list-v1" is the first line of 
the
+  // file. For more details, see
   // 
https://discourse.llvm.org/t/use-glob-instead-of-regex-for-specialcaselists/71666
-  bool UseGlobs = MB->getBuffer().starts_with("#!special-case-list-v2\n");
+  bool UseGlobs = !MB->getBuffer().starts_with("#!special-case-list-v1\n");
 
   for (line_iterator LineIt(*MB, /*SkipBlanks=*/true, /*CommentMarker=*/'#');
!LineIt.is_at_eof(); LineIt++) {
diff --git a/llvm/unittests/Support/SpecialCaseListTest.cpp 
b/llvm/unittests/Support/SpecialCaseListTest.cpp
index 81faeca5d63571..725d20a9b4def7 100644
--- a/llvm/unittests/Support/SpecialCaseListTest.cpp
+++ b/llvm/unittests/Support/SpecialCaseListTest.cpp
@@ -25,8 +25,8 @@ class SpecialCaseListTest : public ::testing::Test {

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[Fangrui Song via cfe-commits]

MaskRay wrote:

> > > Probably would be good to introduce the `-v1` version and require it 
> > > first, then eventually change the default - so people don't get a silent 
> > > behavior change? Even the existing users only using `*` and `.` need to 
> > > change their syntax to migrate to v2, right? They'll need to change `.*` 
> > > to `*` and `.` to `?` and if they don't, their matches will start 
> > > behaving strangely without a specific/actionable error message?
> > 
> > 
> > Since `#!special-case-list-v1` is just a comment and `v1` is the default 
> > before this change, users can actually add it to the first line today and 
> > the behavior won't change until Clang 19.
> 
> Right, but they wouldn't know to do that in advance of the behavior change, 
> would they? If they do nothing/don't read any release notes, they'd get a 
> silent behavior change, I think/if I'm understanding this correctly?

It will be silent but the main idea is that almost all special case lists are 
unaffected by this transition
https://discourse.llvm.org/t/use-glob-instead-of-regex-for-specialcaselists/71666
 and my first comment are to demonstrate this.

I have checked CodeSearch (and ignorelist.txt uses in projects such as 
blue/v8/chromiumos) and actually haven't found any non-glob metacharacters uses 
(`(` `{` etc).

This is understandable: `src:` and `fun:` are the most common entries. `fun:` 
is almost always an exact function name or a glob with a leading/trailing `*`.
Many people tend to think `src:` uses globs and do not escape `.`. We have 
envisioned `src:a.(c|cpp)` (though I haven't seen such a use case) which can be 
satisfied by the glob brace expansion `src:a.{c,cpp}`.

Going forward, glob is the only supported format and we do not want users to 
annotate their ignore list with `#!special-case-list-v2` or `#!glob`.
Enforcing a `#!` header comment (by issuing a warning if not seen) would likely 
cause more disruption.


https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[David Blaikie via cfe-commits]

dwblaikie wrote:

> > Probably would be good to introduce the `-v1` version and require it first, 
> > then eventually change the default - so people don't get a silent behavior 
> > change? Even the existing users only using `*` and `.` need to change their 
> > syntax to migrate to v2, right? They'll need to change `.*` to `*` and `.` 
> > to `?` and if they don't, their matches will start behaving strangely 
> > without a specific/actionable error message?
> 
> Since `#!special-case-list-v1` is just a comment and `v1` is the default 
> before this change, users can actually add it to the first line today and the 
> behavior won't change until Clang 19.

Right, but they wouldn't know to do that in advance of the behavior change, 
would they? If they do nothing/don't read any release notes, they'd get a 
silent behavior change, I think/if I'm understanding this correctly?

https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[Ellis Hoag via cfe-commits]

ellishg wrote:

> Probably would be good to introduce the `-v1` version and require it first, 
> then eventually change the default - so people don't get a silent behavior 
> change? Even the existing users only using `*` and `.` need to change their 
> syntax to migrate to v2, right? They'll need to change `.*` to `*` and `.` to 
> `?` and if they don't, their matches will start behaving strangely without a 
> specific/actionable error message?

Since `#!special-case-list-v1` is just a comment and `v1` is the default before 
this change, users can actually add it to the first line today and the behavior 
won't change until Clang 19.

https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[Petr Hosek via cfe-commits]

https://github.com/petrhosek approved this pull request.


https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)

[David Blaikie via cfe-commits]

dwblaikie wrote:

Probably would be good to introduce the `-v1` version and require it first, 
then eventually change the default - so people don't get a silent behavior 
change? Even the existing users only using `*` and `.` need to change their 
syntax to migrate to v2, right? They'll need to change `.*` to `*` and `.` to 
`?` and if they don't, their matches will start behaving strangely without a 
specific/actionable error message?

https://github.com/llvm/llvm-project/pull/74809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits