[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Basic/VirtualFileSystem.cpp:1391 + error(NameValueNode, +"entry with relative path at the root level is not discoverable"); + return nullptr; Reads better :) https://reviews.llvm.org/D49518 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50118: [VFS] Unify iteration code for VFSFromYamlDirIterImpl, NFC intended.
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for the clean up! LGTM https://reviews.llvm.org/D50118 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50870: Close FileEntries of cached files in ModuleManager::addModule().
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for working on this Adrian, LGTM. > When we last discussed this my plan was to avoid the stat() in > lookupModuleFile() for files that were just added to the PCMCache by > WriteAST() entirely, but ModuleManager::Modules is a DenseMap ModuleFile *> and lookupModuleFile() is the easiest way to create a new > FileEntry. It would be nice to find a way to avoid the stat() for a file that > we just wrote, but it wasn't immediately obvious to me how to do that. Makes sense, I don't know offhand either. It's an extra optimization step we want to do, but it shouldn't block this change. Repository: rL LLVM https://reviews.llvm.org/D50870 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno added a comment. Ping! Repository: rC Clang https://reviews.llvm.org/D46485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D46485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno added a comment. @jkorous, thanks for taking a look. This is an old python script which I haven't touched before, maybe there were answers to your questions but they are now lost in time. I'll address the review though and upload a new version. Repository: rC Clang https://reviews.llvm.org/D46485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno marked an inline comment as done.
bruno added inline comments.
Comment at: utils/hmaptool/hmaptool:55
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
jkorous wrote:
> Wouldn't it be simpler to use modulo 2?
>
> ```
> if num_buckets % 2 == 0
> ```
We want it to be a power of two, not just a multiple.
Comment at: utils/hmaptool/hmaptool:234
+
+if len(args) != 2:
+parser.error("invalid number of arguments")
jkorous wrote:
> Aren't we expecting just single argument () here?
We also need the from --build-path
Repository:
rC Clang
https://reviews.llvm.org/D46485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno updated this revision to Diff 146992.
bruno added a comment.
Update after Jan review.
https://reviews.llvm.org/D46485
Files:
CMakeLists.txt
test/CMakeLists.txt
test/Preprocessor/Inputs/headermap-rel/foo.hmap
test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap.json
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
test/Preprocessor/headermap-rel.c
test/Preprocessor/headermap-rel2.c
test/Preprocessor/nonportable-include-with-hmap.c
utils/hmaptool/CMakeLists.txt
utils/hmaptool/hmaptool
Index: utils/hmaptool/hmaptool
===
--- /dev/null
+++ utils/hmaptool/hmaptool
@@ -0,0 +1,289 @@
+#!/usr/bin/env python
+
+import json
+import optparse
+import os
+import struct
+import sys
+
+###
+
+k_header_magic_LE = 'pamh'
+k_header_magic_BE = 'hmap'
+
+def hmap_hash(str):
+"""hash(str) -> int
+
+Apply the "well-known" headermap hash function.
+"""
+
+return sum((ord(c.lower()) * 13
+for c in str), 0)
+
+class HeaderMap(object):
+@staticmethod
+def frompath(path):
+with open(path, 'rb') as f:
+magic = f.read(4)
+if magic == k_header_magic_LE:
+endian_code = '<'
+elif magic == k_header_magic_BE:
+endian_code = '>'
+else:
+raise SystemExit("error: %s: not a headermap" % (
+path,))
+
+# Read the header information.
+header_fmt = endian_code + 'HH'
+header_size = struct.calcsize(header_fmt)
+data = f.read(header_size)
+if len(data) != header_size:
+raise SystemExit("error: %s: truncated headermap header" % (
+path,))
+
+(version, reserved, strtable_offset, num_entries,
+ num_buckets, max_value_len) = struct.unpack(header_fmt, data)
+
+if version != 1:
+raise SystemExit("error: %s: unknown headermap version: %r" % (
+path, version))
+if reserved != 0:
+raise SystemExit("error: %s: invalid reserved value in header" % (
+path,))
+
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
+path,))
+
+# Read all of the buckets.
+bucket_fmt = endian_code + 'III'
+bucket_size = struct.calcsize(bucket_fmt)
+buckets_data = f.read(num_buckets * bucket_size)
+if len(buckets_data) != num_buckets * bucket_size:
+raise SystemExit("error: %s: truncated headermap buckets" % (
+path,))
+buckets = [struct.unpack(bucket_fmt,
+ buckets_data[i*bucket_size:(i+1)*bucket_size])
+ for i in range(num_buckets)]
+
+# Read the string table; the format doesn't explicitly communicate the
+# size of the string table (which is dumb), so assume it is the rest of
+# the file.
+f.seek(0, 2)
+strtable_size = f.tell() - strtable_offset
+f.seek(strtable_offset)
+
+strtable = f.read(strtable_size)
+if len(strtable) != strtable_size:
+raise SystemExit("error: %s: unable to read complete string table"%(
+path,))
+if strtable[-1] != '\0':
+raise SystemExit("error: %s: invalid string table in headermap" % (
+path,))
+
+return HeaderMap(num_entries, buckets, strtable)
+
+def __init__(self, num_entries, buckets, strtable):
+self.num_entries = num_entries
+self.buckets = buckets
+self.strtable = strtable
+
+def get_string(self, idx):
+if idx >= len(self.strtable):
+raise SystemExit("error: %s: invalid string index" % (
+path,))
+end_idx = self.strtable.index('\0', idx)
+return self.strtable[idx:end_idx]
+
+@property
+def mappings(self):
+for key_idx,prefix_idx,suffix_idx in self.buckets:
+if key_idx == 0:
+continue
+yield (self.get_string(key_idx),
+ self.get_string(prefix_idx) + self.get_string(suffix_idx))
+
+###
+
+def action_dump(name, args):
+"dump a headermap file"
+
+parser = optparse.OptionParser("%%prog %s [options] " % (
+name,))
+parser.add_option("-v", "--verbose", dest="verbose",
+ help="show more verbose output [%default]",
+ actio
[PATCH] D46485: Add python tool to dump and construct header maps
bruno added inline comments. Comment at: test/Preprocessor/headermap-rel.c:2-3 // This uses a headermap with this entry: // Foo.h -> Foo/Foo.h dexonsmith wrote: > Should we delete this comment now that the header map is human-readable? Sure! Comment at: test/Preprocessor/headermap-rel.c:6 +// RUN: rm -f %t.hmap +// RUN: hmaptool write %S/Inputs/headermap-rel/foo.hmap.json %t.hmap +// RUN: %clang_cc1 -E %s -o %t.i -I %t.hmap -F %S/Inputs/headermap-rel dexonsmith wrote: > Should there be some sort of `REQUIRES:` clause for using `hmaptool`? Not really needed, `hmaptool` is setup to work just like `clang-rename` and others, it will always be available to be used by lit. Additionally, it's not importing any module that isn't in the python standard library. https://reviews.llvm.org/D46485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47118: [modules] Print input files when -module-file-info file switch is passed.
bruno added a comment. Thanks for improving this Vassil. Where does in the output it shows up? I wonder if we should have those at the end given the huge amount of headers some modules might have. Repository: rC Clang https://reviews.llvm.org/D47118 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno updated this revision to Diff 147829.
bruno added a comment.
Update testcases (and fixed a missing one) after Duncan's review.
https://reviews.llvm.org/D46485
Files:
CMakeLists.txt
test/CMakeLists.txt
test/Modules/crash-vfs-headermaps.m
test/Preprocessor/Inputs/headermap-rel/foo.hmap
test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap.json
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
test/Preprocessor/headermap-rel.c
test/Preprocessor/headermap-rel2.c
test/Preprocessor/nonportable-include-with-hmap.c
utils/hmaptool/CMakeLists.txt
utils/hmaptool/hmaptool
Index: utils/hmaptool/hmaptool
===
--- /dev/null
+++ utils/hmaptool/hmaptool
@@ -0,0 +1,289 @@
+#!/usr/bin/env python
+
+import json
+import optparse
+import os
+import struct
+import sys
+
+###
+
+k_header_magic_LE = 'pamh'
+k_header_magic_BE = 'hmap'
+
+def hmap_hash(str):
+"""hash(str) -> int
+
+Apply the "well-known" headermap hash function.
+"""
+
+return sum((ord(c.lower()) * 13
+for c in str), 0)
+
+class HeaderMap(object):
+@staticmethod
+def frompath(path):
+with open(path, 'rb') as f:
+magic = f.read(4)
+if magic == k_header_magic_LE:
+endian_code = '<'
+elif magic == k_header_magic_BE:
+endian_code = '>'
+else:
+raise SystemExit("error: %s: not a headermap" % (
+path,))
+
+# Read the header information.
+header_fmt = endian_code + 'HH'
+header_size = struct.calcsize(header_fmt)
+data = f.read(header_size)
+if len(data) != header_size:
+raise SystemExit("error: %s: truncated headermap header" % (
+path,))
+
+(version, reserved, strtable_offset, num_entries,
+ num_buckets, max_value_len) = struct.unpack(header_fmt, data)
+
+if version != 1:
+raise SystemExit("error: %s: unknown headermap version: %r" % (
+path, version))
+if reserved != 0:
+raise SystemExit("error: %s: invalid reserved value in header" % (
+path,))
+
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
+path,))
+
+# Read all of the buckets.
+bucket_fmt = endian_code + 'III'
+bucket_size = struct.calcsize(bucket_fmt)
+buckets_data = f.read(num_buckets * bucket_size)
+if len(buckets_data) != num_buckets * bucket_size:
+raise SystemExit("error: %s: truncated headermap buckets" % (
+path,))
+buckets = [struct.unpack(bucket_fmt,
+ buckets_data[i*bucket_size:(i+1)*bucket_size])
+ for i in range(num_buckets)]
+
+# Read the string table; the format doesn't explicitly communicate the
+# size of the string table (which is dumb), so assume it is the rest of
+# the file.
+f.seek(0, 2)
+strtable_size = f.tell() - strtable_offset
+f.seek(strtable_offset)
+
+strtable = f.read(strtable_size)
+if len(strtable) != strtable_size:
+raise SystemExit("error: %s: unable to read complete string table"%(
+path,))
+if strtable[-1] != '\0':
+raise SystemExit("error: %s: invalid string table in headermap" % (
+path,))
+
+return HeaderMap(num_entries, buckets, strtable)
+
+def __init__(self, num_entries, buckets, strtable):
+self.num_entries = num_entries
+self.buckets = buckets
+self.strtable = strtable
+
+def get_string(self, idx):
+if idx >= len(self.strtable):
+raise SystemExit("error: %s: invalid string index" % (
+path,))
+end_idx = self.strtable.index('\0', idx)
+return self.strtable[idx:end_idx]
+
+@property
+def mappings(self):
+for key_idx,prefix_idx,suffix_idx in self.buckets:
+if key_idx == 0:
+continue
+yield (self.get_string(key_idx),
+ self.get_string(prefix_idx) + self.get_string(suffix_idx))
+
+###
+
+def action_dump(name, args):
+"dump a headermap file"
+
+parser = optparse.OptionParser("%%prog %s [options] " % (
+name,))
+parser.add_option("-v", "--verbose", dest="verbose",
+
[PATCH] D47157: Warning for framework headers using double quote includes
bruno created this revision.
bruno added reviewers: dexonsmith, ributzka, steven_wu.
Introduce -Wquoted-include-in-framework-header, which should fire
a warning whenever a quote include appears in a framework header,
example, for A.h added in the tests:
...
A.framework/Headers/A.h:2:10: warning: double-quoted include "A0.h" in
framework header, expected system style include
This should help users prevent frameworks from using local headers
when in fact they should be targetting system level ones.
The warning is off by default.
rdar://problem/37077034
Repository:
rC Clang
https://reviews.llvm.org/D47157
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/HeaderSearch.cpp
test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
test/Modules/Inputs/double-quotes/B.h
test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
test/Modules/Inputs/double-quotes/a.hmap.json
test/Modules/Inputs/double-quotes/flat-header-path/Z.h
test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
test/Modules/Inputs/double-quotes/x.hmap.json
test/Modules/Inputs/double-quotes/z.yaml
test/Modules/double-quotes.m
Index: test/Modules/double-quotes.m
===
--- /dev/null
+++ test/Modules/double-quotes.m
@@ -0,0 +1,38 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/double-quotes/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/double-quotes/x.hmap.json %t/x.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/double-quotes:g" \
+// RUN: %S/Inputs/double-quotes/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
+// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
+// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s \
+// RUN: 2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: double-quoted include "A0.h" in framework header
+// CHECK: double-quoted include "B.h" in framework header
+// CHECK: double-quoted include "B.h" in framework header
+
+#import "A.h"
+#import
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:2{{double-quoted include "A0.h" in framework header}}
+// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:3{{double-quoted include "B.h" in framework header}}
+// expected-warning@Inputs/double-quotes/flat-header-path/Z.h:1{{double-quoted include "B.h" in framework header}}
Index: test/Modules/Inputs/double-quotes/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/z.yaml
@@ -0,0 +1,28 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Modules",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+}
+ ]
+}
+ ]
+}
Index: test/Modules/Inputs/double-quotes/x.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/x.hmap.json
@@ -0,0 +1,7 @@
+
+{
+ "mappings" :
+{
+ "X.h" : "X/X.h"
+}
+}
Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
@@ -0,0 +1,3 @@
+framework module Z {
+ header "Z.h"
+}
Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.h
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/flat-header-path/Z.h
@@ -0,0 +1 @@
+#import "B.h"
Index: test/Modules/Inputs/double-quotes/a.hmap.json
===
--- /dev/null
+++ t
[PATCH] D47157: Warning for framework headers using double quote includes
bruno added a comment. Depends on https://reviews.llvm.org/D46485 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
[PATCH] D47157: Warning for framework headers using double quote includes
bruno added a comment. > See also PR22165. Nice, seems related to this indeed. Are you aware of any development along those lines in clang-tidy? We would like this to be part of clang and be used as part of the normal compiler workflow, it can surely be as well part of any clang-tidy related include guidelines in the future. >> The warning is off by default. > > We typically do not add off-by-default warnings because experience has shown > the rarely get enabled in practice. Can you explain a bit more about why this > one is off by default? Right. I believe this is going to be used in practice, the reason I'm adding it involves some user demand for such warning. Such quoted include use in frameworks happen often and we would like a smooth transition to happen here (e.g. do not initially affect -Werror users). If it proves worth it, we can migrate to on by default in the future. It wouldn't be a problem if we have it on by default on open source and disable by default downstream, but I rather be consistent. 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
[PATCH] D46485: Add python tool to dump and construct header maps
bruno updated this revision to Diff 147901.
bruno added a comment.
Remove more outdated comments
https://reviews.llvm.org/D46485
Files:
CMakeLists.txt
test/CMakeLists.txt
test/Modules/crash-vfs-headermaps.m
test/Preprocessor/Inputs/headermap-rel/foo.hmap
test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap.json
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
test/Preprocessor/headermap-rel.c
test/Preprocessor/headermap-rel2.c
test/Preprocessor/nonportable-include-with-hmap.c
utils/hmaptool/CMakeLists.txt
utils/hmaptool/hmaptool
Index: utils/hmaptool/hmaptool
===
--- /dev/null
+++ utils/hmaptool/hmaptool
@@ -0,0 +1,289 @@
+#!/usr/bin/env python
+
+import json
+import optparse
+import os
+import struct
+import sys
+
+###
+
+k_header_magic_LE = 'pamh'
+k_header_magic_BE = 'hmap'
+
+def hmap_hash(str):
+"""hash(str) -> int
+
+Apply the "well-known" headermap hash function.
+"""
+
+return sum((ord(c.lower()) * 13
+for c in str), 0)
+
+class HeaderMap(object):
+@staticmethod
+def frompath(path):
+with open(path, 'rb') as f:
+magic = f.read(4)
+if magic == k_header_magic_LE:
+endian_code = '<'
+elif magic == k_header_magic_BE:
+endian_code = '>'
+else:
+raise SystemExit("error: %s: not a headermap" % (
+path,))
+
+# Read the header information.
+header_fmt = endian_code + 'HH'
+header_size = struct.calcsize(header_fmt)
+data = f.read(header_size)
+if len(data) != header_size:
+raise SystemExit("error: %s: truncated headermap header" % (
+path,))
+
+(version, reserved, strtable_offset, num_entries,
+ num_buckets, max_value_len) = struct.unpack(header_fmt, data)
+
+if version != 1:
+raise SystemExit("error: %s: unknown headermap version: %r" % (
+path, version))
+if reserved != 0:
+raise SystemExit("error: %s: invalid reserved value in header" % (
+path,))
+
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
+path,))
+
+# Read all of the buckets.
+bucket_fmt = endian_code + 'III'
+bucket_size = struct.calcsize(bucket_fmt)
+buckets_data = f.read(num_buckets * bucket_size)
+if len(buckets_data) != num_buckets * bucket_size:
+raise SystemExit("error: %s: truncated headermap buckets" % (
+path,))
+buckets = [struct.unpack(bucket_fmt,
+ buckets_data[i*bucket_size:(i+1)*bucket_size])
+ for i in range(num_buckets)]
+
+# Read the string table; the format doesn't explicitly communicate the
+# size of the string table (which is dumb), so assume it is the rest of
+# the file.
+f.seek(0, 2)
+strtable_size = f.tell() - strtable_offset
+f.seek(strtable_offset)
+
+strtable = f.read(strtable_size)
+if len(strtable) != strtable_size:
+raise SystemExit("error: %s: unable to read complete string table"%(
+path,))
+if strtable[-1] != '\0':
+raise SystemExit("error: %s: invalid string table in headermap" % (
+path,))
+
+return HeaderMap(num_entries, buckets, strtable)
+
+def __init__(self, num_entries, buckets, strtable):
+self.num_entries = num_entries
+self.buckets = buckets
+self.strtable = strtable
+
+def get_string(self, idx):
+if idx >= len(self.strtable):
+raise SystemExit("error: %s: invalid string index" % (
+path,))
+end_idx = self.strtable.index('\0', idx)
+return self.strtable[idx:end_idx]
+
+@property
+def mappings(self):
+for key_idx,prefix_idx,suffix_idx in self.buckets:
+if key_idx == 0:
+continue
+yield (self.get_string(key_idx),
+ self.get_string(prefix_idx) + self.get_string(suffix_idx))
+
+###
+
+def action_dump(name, args):
+"dump a headermap file"
+
+parser = optparse.OptionParser("%%prog %s [options] " % (
+name,))
+parser.add_option("-v", "--verbose", dest="verbose",
+ help="show more verbose outpu
[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context
bruno created this revision.
bruno added a reviewer: rsmith.
Not doing so causes the AST writter to assert since the decl in question never
gets emitted. This is fine when modules is not used, but otherwise we need to
serialize something other than garbage.
rdar://problem/39844933
Repository:
rC Clang
https://reviews.llvm.org/D47297
Files:
lib/Sema/SemaDeclObjC.cpp
test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
test/Modules/protocol-redefinition.m
Index: test/Modules/protocol-redefinition.m
===
--- /dev/null
+++ test/Modules/protocol-redefinition.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t
-F%S/Inputs/protocol-redefinition -fsyntax-only %s -Wno-private-module -verify
+
+// expected-no-diagnostics
+
+@import Kit;
Index:
test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
===
--- /dev/null
+++
test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Kit {
+ header "Kit.h"
+ export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
@@ -0,0 +1,6 @@
+#import
+
+// REDECLARATION
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index:
test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
===
--- /dev/null
+++
test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Base {
+ header "Base.h"
+ export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
@@ -0,0 +1,3 @@
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -1202,6 +1202,11 @@
PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName,
ProtocolLoc, AtProtoInterfaceLoc,
/*PrevDecl=*/nullptr);
+
+// If we are using modules, add the decl to the context in order to
+// serialize something meaningful.
+if (getLangOpts().Modules)
+ PushOnScopeChains(PDecl, TUScope);
PDecl->startDefinition();
} else {
if (PrevDecl) {
Index: test/Modules/protocol-redefinition.m
===
--- /dev/null
+++ test/Modules/protocol-redefinition.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F%S/Inputs/protocol-redefinition -fsyntax-only %s -Wno-private-module -verify
+
+// expected-no-diagnostics
+
+@import Kit;
Index: test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Kit {
+ header "Kit.h"
+ export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
@@ -0,0 +1,6 @@
+#import
+
+// REDECLARATION
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index: test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Base {
+ header "Base.h"
+ export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
@@ -0,0 +1,3 @@
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDec
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno created this revision.
bruno added reviewers: ributzka, vsapsai, dexonsmith.
Framework vendors usually layout their framework headers in the following way:
Foo.framework/Headers -> "public" headers
Foo.framework/PrivateHeader -> "private" headers
Since both headers in both directories can be found with #import
, it's easy to make mistakes and include headers in
Foo.framework/PrivateHeader from headers in Foo.framework/Headers, which
usually configures a layering violation on Darwin ecosystems. One of the
problem this causes is dep cycles when modules are used, since it's very common
for "private" modules to include from the "public" ones; adding an edge the
other way around will trigger cycles.
Add a warning to catch those cases such that:
In file included from test.m:1:
./A.framework/Headers/A.h:2:10: warning: public framework header includes
private framework header 'A/APriv.h' [-Wframework-include-private-from-public]
^
This only works for the layering violation within a framework.
rdar://problem/38712182
Depends on https://reviews.llvm.org/D47157
Repository:
rC Clang
https://reviews.llvm.org/D47301
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/HeaderSearch.cpp
test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
test/Modules/Inputs/framework-public-includes-private/a.hmap.json
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
test/Modules/Inputs/framework-public-includes-private/z.hmap.json
test/Modules/Inputs/framework-public-includes-private/z.yaml
test/Modules/framework-public-includes-private.m
Index: test/Modules/framework-public-includes-private.m
===
--- /dev/null
+++ test/Modules/framework-public-includes-private.m
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \
+// RUN: %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s \
+// RUN: 2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: public framework header includes private framework header 'A/APriv.h'
+// CHECK: public framework header includes private framework header 'A/APriv2.h'
+// CHECK: public framework header includes private framework header 'Z/ZPriv.h'
+
+#import "A.h"
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv.h'}}
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}}
+// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}}
Index: test/Modules/Inputs/framework-public-includes-private/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.yaml
@@ -0,0 +1,45 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'use-external-names' : 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno updated this revision to Diff 148317.
bruno added a comment.
Update to a more recent version of the patch.
https://reviews.llvm.org/D47301
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/HeaderSearch.cpp
test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
test/Modules/Inputs/framework-public-includes-private/a.hmap.json
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
test/Modules/Inputs/framework-public-includes-private/z.hmap.json
test/Modules/Inputs/framework-public-includes-private/z.yaml
test/Modules/framework-public-includes-private.m
Index: test/Modules/framework-public-includes-private.m
===
--- /dev/null
+++ test/Modules/framework-public-includes-private.m
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \
+// RUN: %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s \
+// RUN: 2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: public framework header includes private framework header 'A/APriv.h'
+// CHECK: public framework header includes private framework header 'A/APriv2.h'
+// CHECK: public framework header includes private framework header 'Z/ZPriv.h'
+
+#import "A.h"
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv.h'}}
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}}
+// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}}
Index: test/Modules/Inputs/framework-public-includes-private/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.yaml
@@ -0,0 +1,45 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'use-external-names' : 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/PrivateHeaders",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "ZPriv.h",
+ 'external-contents': "TEST_DIR/flat-header-path/ZPriv.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Modules",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+},
+{
+ 'type': 'file',
+ 'name': "module.private.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.private.modulemap"
+}
+ ]
+}
+ ]
+}
Index: test/Modules/Inputs/framework-public-includes-private/z.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.hmap.json
@@ -0,0 +1,7 @@
[PATCH] D47157: Warning for framework headers using double quote includes
bruno added a comment. Hi Eugene, > You could just not include new warning into -Wall and -Wextra. Those who will > want to check #include statements correctness could enable it explicitly. This is exactly what's happening in the patch, the new warning isn't part of `-Wall` or `-Wextra`, and is marked `DefaultIgnore`, which means that will fire only when `-Wquoted-include-in-framework-header` is passed to the driver. Am I missing something from your explanation? Thanks, 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
[PATCH] D46485: Add python tool to dump and construct header maps
bruno added a comment. Ping! https://reviews.llvm.org/D46485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45012: [Modules] Skip adding unused module maps to the dependency file
bruno added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D45012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47157: Warning for framework headers using double quote includes
bruno added a comment. > Consistency would be nice, but at the same time, I don't see a good metric > for when we'd know it's time to switch it to being on by default. I'm worried > that it'll remain off by default forever simply because no one thinks to go > turn it on (because it's silent by default). Perhaps on-by-default here and > off-by-default downstream would be the better approach, or do you think this > would be too disruptive to enable by default anywhere? I believe this could be too disruptive to enable now, since it's still very common to find quoted includes in framework headers. This is very important for Modules to properly work with frameworks (quoted headers are usually considered non-modular when modules builds kick in) and is actually very compelling for us to turn it on by default on Darwin as soon as we can, but we need to educate users first. 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 include" + >, InGroup, DefaultIgnore; 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 `, 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 instead"` How does that sound to you? Other suggestions? Comment at: lib/Lex/HeaderSearch.cpp:753-754 + IncluderAndDir.second->getName())) +Diags.Report(IncludeLoc, + diag::warn_quoted_include_in_framework_header) +<< Filename; 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. 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
[PATCH] D38208: Add support for remembering origins to ExternalASTMerger
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D38208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38364: Fix Modules/builtin-import.mm to be able to handle non-Darwin targets
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks Filipe, LGTM https://reviews.llvm.org/D38364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38868: [OpenCL] Restrict swizzle length check to OpenCL mode
bruno created this revision.
Herald added a subscriber: yaxunl.
Change behavior introduced in r298369 and only error out on vector component
invalid length access on OpenCL mode.
https://reviews.llvm.org/D38868
Files:
lib/Sema/SemaExprMember.cpp
test/SemaOpenCL/vector_swizzle_length.cl
Index: test/SemaOpenCL/vector_swizzle_length.cl
===
--- test/SemaOpenCL/vector_swizzle_length.cl
+++ test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,10 +1,17 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
typedef float float8 __attribute__((ext_vector_type(8)));
void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+#if defined(__OPENCL_C_VERSION__)
f2.s01234; // expected-error {{vector component access has invalid length
5. Supported: 1,2,3,4,8,16}}
f2.xyzxy; // expected-error {{vector component access has invalid length
5. Supported: 1,2,3,4,8,16}}
+#else
+// expected-no-diagnostics
+(void)f2.s01234;
+(void)f2.xyzxy;
+#endif
}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
}
}
- if (!HalvingSwizzle) {
+ // OpenCL mode requires swizzle length to be in accordance with accepted
+ // sizes. Clang however supports arbitrary lengths for other languages.
+ if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
unsigned SwizzleLength = CompName->getLength();
if (HexSwizzle)
Index: test/SemaOpenCL/vector_swizzle_length.cl
===
--- test/SemaOpenCL/vector_swizzle_length.cl
+++ test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,10 +1,17 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
typedef float float8 __attribute__((ext_vector_type(8)));
void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+#if defined(__OPENCL_C_VERSION__)
f2.s01234; // expected-error {{vector component access has invalid length 5. Supported: 1,2,3,4,8,16}}
f2.xyzxy; // expected-error {{vector component access has invalid length 5. Supported: 1,2,3,4,8,16}}
+#else
+// expected-no-diagnostics
+(void)f2.s01234;
+(void)f2.xyzxy;
+#endif
}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
}
}
- if (!HalvingSwizzle) {
+ // OpenCL mode requires swizzle length to be in accordance with accepted
+ // sizes. Clang however supports arbitrary lengths for other languages.
+ if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
unsigned SwizzleLength = CompName->getLength();
if (HexSwizzle)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38868: [OpenCL] Restrict swizzle length check to OpenCL mode
bruno marked 2 inline comments as done.
bruno added inline comments.
Comment at: test/SemaOpenCL/vector_swizzle_length.cl:7
void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
Anastasia wrote:
> Even though this works in Clang, ideally OpenCL vector literal is with
> parenthesis (see s6.1.6).
Ok. I changed this to avoid warnings in C mode. Gonna change it back,
Comment at: test/SemaOpenCL/vector_swizzle_length.cl:13
+#else
+// expected-no-diagnostics
+(void)f2.s01234;
Anastasia wrote:
> I am not sure it's good idea to test C mode here since this is intended for
> OpenCL only functionality. I think it might be better to separate C
> functionality and keep under regular Sema tests. We could keep the same test
> name to be able to identify similar functionality.
Ok!
https://reviews.llvm.org/D38868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38868: [OpenCL] Restrict swizzle length check to OpenCL mode
bruno updated this revision to Diff 118966.
bruno marked 2 inline comments as done.
bruno added a comment.
Update patch after Anastasia's suggestions
https://reviews.llvm.org/D38868
Files:
lib/Sema/SemaExprMember.cpp
test/Sema/vector_swizzle_length.c
test/SemaOpenCL/vector_swizzle_length.cl
Index: test/SemaOpenCL/vector_swizzle_length.cl
===
--- test/SemaOpenCL/vector_swizzle_length.cl
+++ test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,9 +1,9 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
typedef float float8 __attribute__((ext_vector_type(8)));
void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
f2.s01234; // expected-error {{vector component access has invalid length
5. Supported: 1,2,3,4,8,16}}
f2.xyzxy; // expected-error {{vector component access has invalid length
5. Supported: 1,2,3,4,8,16}}
Index: test/Sema/vector_swizzle_length.c
===
--- /dev/null
+++ test/Sema/vector_swizzle_length.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef float float8 __attribute__((ext_vector_type(8)));
+
+void foo() {
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+(void)f2.s01234;
+(void)f2.xyzxy;
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
}
}
- if (!HalvingSwizzle) {
+ // OpenCL mode requires swizzle length to be in accordance with accepted
+ // sizes. Clang however supports arbitrary lengths for other languages.
+ if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
unsigned SwizzleLength = CompName->getLength();
if (HexSwizzle)
Index: test/SemaOpenCL/vector_swizzle_length.cl
===
--- test/SemaOpenCL/vector_swizzle_length.cl
+++ test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,9 +1,9 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
typedef float float8 __attribute__((ext_vector_type(8)));
void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
f2.s01234; // expected-error {{vector component access has invalid length 5. Supported: 1,2,3,4,8,16}}
f2.xyzxy; // expected-error {{vector component access has invalid length 5. Supported: 1,2,3,4,8,16}}
Index: test/Sema/vector_swizzle_length.c
===
--- /dev/null
+++ test/Sema/vector_swizzle_length.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef float float8 __attribute__((ext_vector_type(8)));
+
+void foo() {
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+(void)f2.s01234;
+(void)f2.xyzxy;
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
}
}
- if (!HalvingSwizzle) {
+ // OpenCL mode requires swizzle length to be in accordance with accepted
+ // sizes. Clang however supports arbitrary lengths for other languages.
+ if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
unsigned SwizzleLength = CompName->getLength();
if (HexSwizzle)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38868: [OpenCL] Restrict swizzle length check to OpenCL mode
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316016: [OpenCL] Restrict swizzle length check to OpenCL
mode (authored by bruno).
Changed prior to commit:
https://reviews.llvm.org/D38868?vs=118966&id=119351#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38868
Files:
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/test/Sema/vector_swizzle_length.c
cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
Index: cfe/trunk/test/Sema/vector_swizzle_length.c
===
--- cfe/trunk/test/Sema/vector_swizzle_length.c
+++ cfe/trunk/test/Sema/vector_swizzle_length.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef float float8 __attribute__((ext_vector_type(8)));
+
+void foo() {
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+(void)f2.s01234;
+(void)f2.xyzxy;
+}
Index: cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
===
--- cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
+++ cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
typedef float float8 __attribute__((ext_vector_type(8)));
Index: cfe/trunk/lib/Sema/SemaExprMember.cpp
===
--- cfe/trunk/lib/Sema/SemaExprMember.cpp
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
}
}
- if (!HalvingSwizzle) {
+ // OpenCL mode requires swizzle length to be in accordance with accepted
+ // sizes. Clang however supports arbitrary lengths for other languages.
+ if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
unsigned SwizzleLength = CompName->getLength();
if (HexSwizzle)
Index: cfe/trunk/test/Sema/vector_swizzle_length.c
===
--- cfe/trunk/test/Sema/vector_swizzle_length.c
+++ cfe/trunk/test/Sema/vector_swizzle_length.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef float float8 __attribute__((ext_vector_type(8)));
+
+void foo() {
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+(void)f2.s01234;
+(void)f2.xyzxy;
+}
Index: cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
===
--- cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
+++ cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
typedef float float8 __attribute__((ext_vector_type(8)));
Index: cfe/trunk/lib/Sema/SemaExprMember.cpp
===
--- cfe/trunk/lib/Sema/SemaExprMember.cpp
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
}
}
- if (!HalvingSwizzle) {
+ // OpenCL mode requires swizzle length to be in accordance with accepted
+ // sizes. Clang however supports arbitrary lengths for other languages.
+ if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
unsigned SwizzleLength = CompName->getLength();
if (HexSwizzle)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51910: [Modules] Add platform feature to requires clause
bruno created this revision.
bruno added reviewers: rsmith, v.g.vassilev, aprantl.
Herald added subscribers: dexonsmith, srhines.
Allows module map writers to add build requirements based on platform/os.
Useful when target features and language dialects aren't enough to
conditionalize building a module. Among other things, it also makes it more
convenient for modules with the same name but different set of submodules to be
described in the same module map.
rdar://problem/43909745
Repository:
rC Clang
https://reviews.llvm.org/D51910
Files:
docs/Modules.rst
lib/Basic/Module.cpp
test/Modules/target-platform-features.m
Index: test/Modules/target-platform-features.m
===
--- /dev/null
+++ test/Modules/target-platform-features.m
@@ -0,0 +1,33 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/Inputs
+
+// Build module map file
+// RUN: echo "module RequiresMacOS {" >> %t/Inputs/module.map
+// RUN: echo " requires macos" >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo " requires !ios" >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresMain {" >> %t/Inputs/module.map
+// RUN: echo " module SubRequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo "requires !ios" >> %t/Inputs/module.map
+// RUN: echo " }">> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+
+// RUN: %clang_cc1 -triple=x86_64-apple-macosx10.6 -fmodules
-fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/Inputs
-verify %s -fblocks -fobjc-arc
+// expected-no-diagnostics
+
+// RUN: not %clang_cc1 -triple=arm64-apple-ios -fmodules
-fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/Inputs
%s -fblocks -fobjc-arc 2> %t.notios
+// RUN: FileCheck %s -check-prefix=CHECK-IOS < %t.notios
+
+// CHECK-IOS: module 'RequiresMacOS' requires feature 'macos'
+@import RequiresMacOS;
+// CHECK-IOS: module 'RequiresNotiOS' is incompatible with feature 'ios'
+@import RequiresNotiOS;
+
+// We should never get errors for submodules that don't match
+// CHECK-IOS-NOT: module 'RequiresMain'
+@import RequiresMain;
Index: lib/Basic/Module.cpp
===
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -93,7 +93,8 @@
.Case("opencl", LangOpts.OpenCL)
.Case("tls", Target.isTLSSupported())
.Case("zvector", LangOpts.ZVector)
-.Default(Target.hasFeature(Feature));
+.Default(Target.hasFeature(Feature) ||
+ Target.getPlatformName() == Feature);
if (!HasFeature)
HasFeature = std::find(LangOpts.ModuleFeatures.begin(),
LangOpts.ModuleFeatures.end(),
Index: docs/Modules.rst
===
--- docs/Modules.rst
+++ docs/Modules.rst
@@ -466,6 +466,8 @@
*target feature*
A specific target feature (e.g., ``sse4``, ``avx``, ``neon``) is available.
+*platform variant*
+ A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``,
``win32``, ``linux``, etc) is available.
**Example:** The ``std`` module can be extended to also include C++ and C++11
headers using a *requires-declaration*:
Index: test/Modules/target-platform-features.m
===
--- /dev/null
+++ test/Modules/target-platform-features.m
@@ -0,0 +1,33 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/Inputs
+
+// Build module map file
+// RUN: echo "module RequiresMacOS {" >> %t/Inputs/module.map
+// RUN: echo " requires macos" >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo " requires !ios" >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresMain {" >> %t/Inputs/module.map
+// RUN: echo " module SubRequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo "requires !ios" >> %t/Inputs/module.map
+// RUN: echo " }">> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+
+// RUN: %clang_cc1 -triple=x86_64-apple-macosx10.6 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/Inputs -verify %s -fblocks -fobjc-arc
+// expected-no-diagnostics
+
+// RUN: not %clang_cc1 -triple=arm64-apple-ios -fmodules -fimplicit-module-maps -fmodules-cach
[PATCH] D51910: [Modules] Add platform feature to requires clause
bruno updated this revision to Diff 165612.
bruno added a comment.
Addressed Adrian's review. Added support to consider the environment and well
the combination platform-environment. @aprantl also handled multiple variants
of simulator combinations.
https://reviews.llvm.org/D51910
Files:
docs/Modules.rst
lib/Basic/Module.cpp
lib/Lex/ModuleMap.cpp
test/Modules/target-platform-features.m
Index: test/Modules/target-platform-features.m
===
--- /dev/null
+++ test/Modules/target-platform-features.m
@@ -0,0 +1,83 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/InputsA
+
+// RUN: echo "module RequiresMacOS {" >> %t/InputsA/module.map
+// RUN: echo " requires macos" >> %t/InputsA/module.map
+// RUN: echo "}">> %t/InputsA/module.map
+// RUN: echo "module RequiresNotiOS {" >> %t/InputsA/module.map
+// RUN: echo " requires !ios" >> %t/InputsA/module.map
+// RUN: echo "}">> %t/InputsA/module.map
+// RUN: echo "module RequiresMain {">> %t/InputsA/module.map
+// RUN: echo " module SubRequiresNotiOS {" >> %t/InputsA/module.map
+// RUN: echo "requires !ios">> %t/InputsA/module.map
+// RUN: echo " }" >> %t/InputsA/module.map
+// RUN: echo "}">> %t/InputsA/module.map
+
+// RUN: %clang_cc1 -triple=x86_64-apple-macosx10.6 -DENABLE_DARWIN -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsA -verify %s
+// expected-no-diagnostics
+
+// RUN: not %clang_cc1 -triple=arm64-apple-ios -DENABLE_DARWIN -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsA %s 2> %t/notios
+// RUN: FileCheck %s -check-prefix=CHECK-IOS < %t/notios
+#ifdef ENABLE_DARWIN
+// CHECK-IOS: module 'RequiresMacOS' requires feature 'macos'
+@import RequiresMacOS;
+// CHECK-IOS: module 'RequiresNotiOS' is incompatible with feature 'ios'
+@import RequiresNotiOS;
+// We should never get errors for submodules that don't match
+// CHECK-IOS-NOT: module 'RequiresMain'
+@import RequiresMain;
+#endif
+
+// RUN: mkdir %t/InputsB
+// RUN: echo "module RequiresiOSSimA {" >> %t/InputsB/module.map
+// RUN: echo " requires iossimulator" >> %t/InputsB/module.map
+// RUN: echo "}">> %t/InputsB/module.map
+// RUN: echo "module RequiresiOSSimB {" >> %t/InputsB/module.map
+// RUN: echo " requires ios-simulator" >> %t/InputsB/module.map
+// RUN: echo "}">> %t/InputsB/module.map
+// RUN: %clang_cc1 -triple=x86_64-apple-iossimulator -DENABLE_IOSSIM -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsB %s -verify
+// RUN: %clang_cc1 -triple=x86_64-apple-ios-simulator -DENABLE_IOSSIM -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsB %s -verify
+
+#ifdef ENABLE_IOSSIM
+@import RequiresiOSSimA;
+@import RequiresiOSSimB;
+#endif
+
+// RUN: mkdir %t/InputsC
+// RUN: echo "module RequiresLinuxEABIA {" >> %t/InputsC/module.map
+// RUN: echo " requires linux-gnueabi" >> %t/InputsC/module.map
+// RUN: echo "}">> %t/InputsC/module.map
+// RUN: echo "module RequiresLinuxEABIB {" >> %t/InputsC/module.map
+// RUN: echo " requires gnueabi" >> %t/InputsC/module.map
+// RUN: echo "}">> %t/InputsC/module.map
+// RUN: echo "module RequiresLinuxEABIC {" >> %t/InputsC/module.map
+// RUN: echo " requires linux" >> %t/InputsC/module.map
+// RUN: echo "}">> %t/InputsC/module.map
+// RUN: %clang_cc1 -triple=armv8r-none-linux-gnueabi -DENABLE_LINUXEABI -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsC %s -verify
+
+#ifdef ENABLE_LINUXEABI
+@import RequiresLinuxEABIA;
+@import RequiresLinuxEABIB;
+@import RequiresLinuxEABIC;
+#endif
+
+// RUN: mkdir %t/InputsD
+// RUN: echo "module RequiresWinMSVCA {" >> %t/InputsD/module.map
+// RUN: echo " requires windows" >> %t/InputsD/module.map
+// RUN: echo "}" >> %t/InputsD/module.map
+// RUN: echo "module RequiresWinMSVCB {" >> %t/InputsD/module.map
+// RUN: echo " requires windows-msvc">> %t/InputsD/module.map
+// RUN: echo "}" >> %t/InputsD/module.map
+// RUN: echo "module RequiresWinMSVCC {" >> %t/InputsD/module.map
+// RUN: echo " requires msvc">> %t/InputsD/module.map
+// RUN: echo "}" >> %t/InputsD/module.map
+// RUN: %clang_cc1 -triple=thumbv7-unknown-windows-msvc -DENABLE_WINMSVC -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsD %s -verify
+
+#ifdef ENABLE_WINMSVC
+@import RequiresWinMSVCA;
+@import RequiresWinMSVCB;
+@impor
[PATCH] D48685: [PCH+Modules] Load -fmodule-map-file content before including PCHs
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337447: [PCH+Modules] Load -fmodule-map-file content before
including PCHs (authored by bruno, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48685?vs=153205&id=156247#toc
Repository:
rL LLVM
https://reviews.llvm.org/D48685
Files:
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
Index: cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
===
--- cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
+++ cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dst %t.cache
+// RUN: mkdir -p %t.dst/folder-with-modulemap %t.dst/pch-folder
+// RUN: echo '#import "folder-with-modulemap/included.h"' > %t.dst/header.h
+// RUN: echo 'extern int MyModuleVersion;' >
%t.dst/folder-with-modulemap/MyModule.h
+// RUN: echo '@import MyModule;' > %t.dst/folder-with-modulemap/included.h
+// RUN: echo 'module MyModule { header "MyModule.h" }' >
%t.dst/folder-with-modulemap/MyModule.modulemap
+
+// RUN: %clang_cc1 -emit-pch -o %t.dst/pch-folder/header.pch
-fmodule-map-file=%t.dst/folder-with-modulemap/MyModule.modulemap -x
objective-c-header -fmodules-cache-path=%t.cache -fmodules
-fimplicit-module-maps %t.dst/header.h
+// RUN: %clang_cc1 -fsyntax-only
-fmodule-map-file=%t.dst/folder-with-modulemap/MyModule.modulemap
-fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps %s -include-pch
%t.dst/pch-folder/header.pch -verify
+
+// expected-no-diagnostics
+
+void test() {
+ (void)MyModuleVersion; // should be found by implicit import
+}
+
Index: cfe/trunk/lib/Frontend/FrontendAction.cpp
===
--- cfe/trunk/lib/Frontend/FrontendAction.cpp
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp
@@ -766,6 +766,22 @@
if (!BeginSourceFileAction(CI))
goto failure;
+ // If we were asked to load any module map files, do so now.
+ for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+if (auto *File = CI.getFileManager().getFile(Filename))
+ CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+ File, /*IsSystem*/false);
+else
+ CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+ }
+
+ // Add a module declaration scope so that modules from -fmodule-map-file
+ // arguments may shadow modules found implicitly in search paths.
+ CI.getPreprocessor()
+ .getHeaderSearchInfo()
+ .getModuleMap()
+ .finishModuleDeclarationScope();
+
// Create the AST context and consumer unless this is a preprocessor only
// action.
if (!usesPreprocessorOnly()) {
@@ -855,22 +871,6 @@
"doesn't support modules");
}
- // If we were asked to load any module map files, do so now.
- for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto *File = CI.getFileManager().getFile(Filename))
- CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
- File, /*IsSystem*/false);
-else
- CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
- }
-
- // Add a module declaration scope so that modules from -fmodule-map-file
- // arguments may shadow modules found implicitly in search paths.
- CI.getPreprocessor()
- .getHeaderSearchInfo()
- .getModuleMap()
- .finishModuleDeclarationScope();
-
// If we were asked to load any module files, do so now.
for (const auto &ModuleFile : CI.getFrontendOpts().ModuleFiles)
if (!CI.loadModuleFile(ModuleFile))
Index: cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
===
--- cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
+++ cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dst %t.cache
+// RUN: mkdir -p %t.dst/folder-with-modulemap %t.dst/pch-folder
+// RUN: echo '#import "folder-with-modulemap/included.h"' > %t.dst/header.h
+// RUN: echo 'extern int MyModuleVersion;' > %t.dst/folder-with-modulemap/MyModule.h
+// RUN: echo '@import MyModule;' > %t.dst/folder-with-modulemap/included.h
+// RUN: echo 'module MyModule { header "MyModule.h" }' > %t.dst/folder-with-modulemap/MyModule.modulemap
+
+// RUN: %clang_cc1 -emit-pch -o %t.dst/pch-folder/header.pch -fmodule-map-file=%t.dst/folder-with-modulemap/MyModule.modulemap -x objective-c-header -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps %t.dst/header.h
+// RUN: %clang_cc1 -fsyntax-only -fmodule-map-file=%t.dst/folder-with-modulemap/MyModule.modulemap -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps %s -include-pch %t.dst/pch-folder/header.pch -verify
+
+// expected-no-diagnostics
+
+void test(
[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.
bruno accepted this revision. bruno added a comment. Thanks for working on this. LGTM with improvements in the comments as mentioned by @jkorous https://reviews.llvm.org/D48786 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.
bruno added a comment. Hi Volodymyr, thanks for improving this. > Need to double check what tests we have when using relative path names at the > root level. Another interesting place to look at is `unittests/Basic/VirtualFileSystemTest.cpp` > I'd like to make the behavior consistent because a file name is a specific > case of relative paths. So far there are no assertions and no errors but file > lookup doesn't seem to be working. Makes sense. My general impression is that relative paths don't make sense here either, but you can address that in a follow up patch (giving time for any potential counterexample on this). Comment at: clang/lib/Basic/VirtualFileSystem.cpp:1416 +if (NameValueNode) + error(NameValueNode, "file is not located in any directory"); +return nullptr; vsapsai wrote: > Not happy with the error message but didn't come up with anything better. > Suggestions are welcome. What about the message you used in the comment: `file entry is at the root level, outside of any directory` ? https://reviews.llvm.org/D49518 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43128: Introduce an API for LLDB to compute the default module cache path
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D43128 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32828: [Modules] Fix conservative assertion for import diagnostics
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303705: [Modules] Fix overly conservative assertion for
import diagnostic (authored by bruno).
Changed prior to commit:
https://reviews.llvm.org/D32828?vs=100016&id=100023#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32828
Files:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h
cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap
cfe/trunk/test/Modules/diagnose-missing-import.m
Index: cfe/trunk/test/Modules/diagnose-missing-import.m
===
--- cfe/trunk/test/Modules/diagnose-missing-import.m
+++ cfe/trunk/test/Modules/diagnose-missing-import.m
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t
-I%S/Inputs/diagnose-missing-import \
+// RUN: -Werror=implicit-function-declaration -fsyntax-only \
+// RUN: -fimplicit-module-maps -verify %s
+@import NCI;
+
+void foo() {
+ XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); //
expected-error {{implicit declaration of function 'XYZLogEvent'}}
expected-error {{declaration of 'XYZLogEvent' must be imported}} expected-error
{{declaration of 'xyzRiskyCloseOpenParam' must be imported from module
'NCI.A'}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be
imported from module 'NCI.A'}}
+}
+
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration
is here}}
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration
is here}}
+// expected-note@Inputs/diagnose-missing-import/a.h:6 {{previous declaration
is here}}
+
Index: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap
===
--- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap
+++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap
@@ -0,0 +1,3 @@
+module NCI {
+ explicit module A { header "a.h" }
+}
Index: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h
===
--- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h
+++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h
@@ -0,0 +1,8 @@
+#ifndef A_h
+#define A_h
+
+@class NSString;
+static NSString * const xyzRiskyCloseOpenParam = @"riskyCloseParam";
+static inline void XYZLogEvent(NSString* eventName, NSString* params);
+
+#endif
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -16097,7 +16097,8 @@
void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc,
Module *Mod) {
// Bail if we're not allowed to implicitly import a module here.
- if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery)
+ if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery ||
+ VisibleModules.isVisible(Mod))
return;
// Create the implicit import declaration.
Index: cfe/trunk/lib/Sema/SemaLookup.cpp
===
--- cfe/trunk/lib/Sema/SemaLookup.cpp
+++ cfe/trunk/lib/Sema/SemaLookup.cpp
@@ -4933,8 +4933,6 @@
void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
MissingImportKind MIK, bool Recover) {
- assert(!isVisible(Decl) && "missing import for non-hidden decl?");
-
// Suggest importing a module providing the definition of this entity, if
// possible.
NamedDecl *Def = getDefinitionToImport(Decl);
Index: cfe/trunk/test/Modules/diagnose-missing-import.m
===
--- cfe/trunk/test/Modules/diagnose-missing-import.m
+++ cfe/trunk/test/Modules/diagnose-missing-import.m
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/diagnose-missing-import \
+// RUN: -Werror=implicit-function-declaration -fsyntax-only \
+// RUN: -fimplicit-module-maps -verify %s
+@import NCI;
+
+void foo() {
+ XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{implicit declaration of function 'XYZLogEvent'}} expected-error {{declaration of 'XYZLogEvent' must be imported}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}}
+}
+
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
+// expected-note@Inputs/diagnose-missing-import/a.h:6 {{previous declaration is here}}
+
Index: cfe/trunk/test/Mod
[PATCH] D33357: Avoid calling report_fatal_error in the destructor of raw_fd_ostream when saving a module timestamp file
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Makes sense. LGTM! Repository: rL LLVM https://reviews.llvm.org/D33357 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33703: Support lazy stat'ing of files referenced by module maps
bruno added a comment.
Hi Richard,
Thanks for improving this further!
Questions / comments:
- I've noticed in the patch that on the ASTWriter side we serialize the
introduced size / mtime, but there are no changes to the ASTReader, so I assume
in the reader side you still need the module map around to fetch those infos,
right?
- Does this patch provide any benefits for the case where we don't provide a
size / mtime for the header in the module map? Since Size / ModTime are only
updated when parsing the module map I would assume no, but I might have missed
something.
Comment at: include/clang/Basic/DiagnosticSerializationKinds.td:193
+ "cannot emit module %0: %select{size|mtime}1 must be explicitly specified "
+ "for missing header file \"%2\"">;
} // let CategoryName
Can you add a testcase that test this error?
Repository:
rL LLVM
https://reviews.llvm.org/D33703
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32520: Support __fp16 vectors
bruno added a comment.
Hi Akira,
This is nice, thanks for doing it!
Comment at: include/clang/Sema/Sema.h:9270
+ QualType RHSType,
+ bool CompAssign = false);
Can you change `CompAssign` to `IsCompAssign` so that we match the pattern used
in other methods?
Comment at: include/clang/Sema/Sema.h:9279
+ bool ConvertRHS = true,
+ bool CompAssign = false);
Same here.
Comment at: lib/CodeGen/CGExprScalar.cpp:997
- // Allow bitcast from vector to integer/fp of the same size.
- if (isa(SrcTy) ||
- isa(DstTy))
-return Builder.CreateBitCast(Src, DstTy, "conv");
+ if (isa(SrcTy) || isa(DstTy)) {
+auto GetSize = [](const llvm::Type *Ty) {
Please also add a comment explaining what's going on here, like we see for
other snippets of logic above.
It also sounds like this is more generic than it should (which can have
unexpected side effects due to the lack of testcases covering vector with other
element sizes). I suggest you either (a) add testcases for other sizes or (b)
make the condition more restrictive to be sure you're only changing the logic
you intend to (i.e., half and i16).
After these changes, if it makes sense, can you refactor the logic under this
condition into its own function? Seems like this function is too big already.
Comment at: lib/Sema/SemaExpr.cpp:11433
ExprObjectKind OK = OK_Ordinary;
+ bool ConvertHalfVec = false;
Assuming we're able to handle other vector types here, is it in
`ConvertHalfVec` really necessary? It seems odd to me that we need to special
case it in every case below.
Comment at: test/Sema/fp16vec-sema.c:47
+ // clang currently disallows using these operators on vectors, which is
+ // allowed by gcc.
+ sv0 = !hv0; // expected-error{{invalid argument type}}
Can you add a FIXME here?
https://reviews.llvm.org/D32520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33719: Add _Float16 as a C/C++ source language type
bruno added a comment. Hi Sjoerd, Thanks for working on this. Can you add context to your patch? Comment at: test/CodeGenCXX/float16-declarations-error.cpp:1 +// RUN: not %clang -S -emit-llvm --target=aarch64 %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR + You don't need codegen to test this. Please move this test to test/Lexer, (see test/Lexer/half-literal.cpp). Also, is `--target=aarch64` really needed here? I don't see anything ARM specific in this patch (maybe because I'm missing more context on the patch?). Comment at: test/CodeGenCXX/float16-declarations.cpp:59 +func1t(f1l) + s1.mem2 - f1n + f2n; +#if (__cplusplus >= 201103L) + auto f5l = -1.f16, *f6l = &f2l, f7l = func1t(f3l); Please use `-std=c++11` in the RUN line on top of the file and remove the `#if` here. Can you provide a more reduced testcase and try to put the CHECK lines closer to the statements you want to check? The same ARM question from the comment above also applies here. https://reviews.llvm.org/D33719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33703: Support lazy stat'ing of files referenced by module maps
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D33703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33788: Return a canonical path from getClangResourcePath()
bruno added a reviewer: bruno. bruno added a comment. Hi Aaron, Nice catch! Any chance you can add a testcase to this? https://reviews.llvm.org/D33788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33788: Return a canonical path from getClangResourcePath()
bruno added a comment. > I'm unaware of any other API that canonicalizes the path, which is what users > of this API are going to expect. You can also call `sys::path::remove_dots(Path, /*remove_dot_dot=*/true);` > There's already a test case that covers this: Index/pch-from-libclang.c -- it > was failing on OS X 10.6 for us. If you have a better test case in mind, I > can certainly add it. Works for me. https://reviews.llvm.org/D33788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33788: Return a canonical path from getClangResourcePath()
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. > Yes, but that does not canonicalize the path. For instance, it won't handle > doubled-up slashes or symlinks. Right. > Ultimately, the value returned from this API gets passed to POSIX functions > which expect a canonical path. I don't think `remove_dots()` is sufficient. > It should do the same thing as `getMainExecutable()`, I believe. I see, LGTM then https://reviews.llvm.org/D33788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33732: Catch invalid bitwise operation on vector of floats
bruno added a comment.
Hi,
Comment at: test/Sema/types.c:92
+
+typedef float __attribute__((ext_vector_type(4))) float4;
+float4 test3(float4 x) {
Can you also add a test for the `vector_type` variant? It might be more
appropriate to put this at test/Sema/ext_vector* and test/Sema/vector*
https://reviews.llvm.org/D33732
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33719: Add _Float16 as a C/C++ source language type
bruno added a comment. > About the tests and using --target=aarch64: you're right that there should > nothing be ARM specific here, but it is just that for Aarch64 it will show > "half" IR types which I preferred, while it looks like x86 way of dealing > with is to convert it and work on i16 types. Perhaps I need tests for both? The ideal is to have x86 and ARM tests when testing for the codegen part of it. Keep in mind that you need "REQUIRES: -registered-target" for those. https://reviews.llvm.org/D33719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33732: Catch invalid bitwise operation on vector of floats
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: test/Sema/ext_vector_ops.c:22
+ int array2[17];
+ // FIXME: error message below needs type!
+ (void)(array2[v2ua]); // expected-error{{array subscript is not an integer}}
Can you file a PR for this?
https://reviews.llvm.org/D33732
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34377: Support MS builtins using 'long' on Darwin/LP64
bruno added inline comments. Comment at: include/clang/Basic/Builtins.def:55 // W -> int64_t +// l -> 'int' if builtin is a MS extensions and the target is Darwin/LP64. +// Defaults to 'L' otherwise. rnk wrote: > majnemer wrote: > > Why not just LP64? Seems arbitrary to make this Darwin sensitive. > Every existing prefix is upper case. Do you think it makes it more readable > to follow the pattern? Maybe it isn't worth it. At first attempt I tried to make it more generic, but there seems to be different expected behavior in other platforms: for instance, there are Linux/LP64 tests that expect 'long' here to be 64-bits, see CodeGen/ms-intrinsics-rotations.c. Comment at: include/clang/Basic/Builtins.def:55 // W -> int64_t +// l -> 'int' if builtin is a MS extensions and the target is Darwin/LP64. +// Defaults to 'L' otherwise. bruno wrote: > rnk wrote: > > majnemer wrote: > > > Why not just LP64? Seems arbitrary to make this Darwin sensitive. > > Every existing prefix is upper case. Do you think it makes it more readable > > to follow the pattern? Maybe it isn't worth it. > At first attempt I tried to make it more generic, but there seems to be > different expected behavior in other platforms: for instance, there are > Linux/LP64 tests that expect 'long' here to be 64-bits, see > CodeGen/ms-intrinsics-rotations.c. It might be better indeed, but seems like all "good letters" are taken, what about 'N'? As for "Narrow" long (Duncan's suggestion)? Comment at: lib/AST/ASTContext.cpp:8551 + break; +} case 'W': rnk wrote: > compnerd wrote: > > I agree with @majnemer. Why not base this on the Int64Type? > I'd suggest this code: > IsSpecialLong = true; > // Use "long" if is 32 bits. This prefix is used by intrinsics that need > 32-bit types on LP64 platforms, but need to use "long" in the prototype on > LLP64 platforms like Win64. > if (Context.getTargetInfo().getLongWidth() == 32) > HowLong = 1; > break; See below. Comment at: lib/AST/ASTContext.cpp:8551 + break; +} case 'W': bruno wrote: > rnk wrote: > > compnerd wrote: > > > I agree with @majnemer. Why not base this on the Int64Type? > > I'd suggest this code: > > IsSpecialLong = true; > > // Use "long" if is 32 bits. This prefix is used by intrinsics that need > > 32-bit types on LP64 platforms, but need to use "long" in the prototype on > > LLP64 platforms like Win64. > > if (Context.getTargetInfo().getLongWidth() == 32) > > HowLong = 1; > > break; > See below. I tried something similar before, but I get two tests failing CodeGen/ms-intrinsics-rotations.c and CodeGen/pr27892.c. Your suggestion hits the same failing tests. Both fails because of the Linux issue mentioned above: i32 codegen where i64 is expected. Of course I could improve the condition to handle Linux, but at that point I just thing it's better to use Darwin, which is what the fix is towards anyway. Additional ideas? https://reviews.llvm.org/D34377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34377: Support MS builtins using 'long' on Darwin/LP64
bruno added inline comments. Comment at: lib/AST/ASTContext.cpp:8551 + break; +} case 'W': majnemer wrote: > bruno wrote: > > bruno wrote: > > > rnk wrote: > > > > compnerd wrote: > > > > > I agree with @majnemer. Why not base this on the Int64Type? > > > > I'd suggest this code: > > > > IsSpecialLong = true; > > > > // Use "long" if is 32 bits. This prefix is used by intrinsics that > > > > need 32-bit types on LP64 platforms, but need to use "long" in the > > > > prototype on LLP64 platforms like Win64. > > > > if (Context.getTargetInfo().getLongWidth() == 32) > > > > HowLong = 1; > > > > break; > > > See below. > > I tried something similar before, but I get two tests failing > > CodeGen/ms-intrinsics-rotations.c and CodeGen/pr27892.c. Your suggestion > > hits the same failing tests. Both fails because of the Linux issue > > mentioned above: i32 codegen where i64 is expected. Of course I could > > improve the condition to handle Linux, but at that point I just thing it's > > better to use Darwin, which is what the fix is towards anyway. Additional > > ideas? > I don't think we should sweep this under the rug just because there are some > test failures. There is probably some latent bug worth investigating. Well, there's specific testing for this behavior under Linux, so I assume someone needs this? I don't see how this is sweeping stuff under the rug. https://reviews.llvm.org/D34377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34377: Support MS builtins using 'long' on Darwin/LP64
bruno added inline comments. Comment at: lib/AST/ASTContext.cpp:8551 + break; +} case 'W': bruno wrote: > rnk wrote: > > majnemer wrote: > > > bruno wrote: > > > > bruno wrote: > > > > > rnk wrote: > > > > > > compnerd wrote: > > > > > > > I agree with @majnemer. Why not base this on the Int64Type? > > > > > > I'd suggest this code: > > > > > > IsSpecialLong = true; > > > > > > // Use "long" if is 32 bits. This prefix is used by intrinsics > > > > > > that need 32-bit types on LP64 platforms, but need to use "long" in > > > > > > the prototype on LLP64 platforms like Win64. > > > > > > if (Context.getTargetInfo().getLongWidth() == 32) > > > > > > HowLong = 1; > > > > > > break; > > > > > See below. > > > > I tried something similar before, but I get two tests failing > > > > CodeGen/ms-intrinsics-rotations.c and CodeGen/pr27892.c. Your > > > > suggestion hits the same failing tests. Both fails because of the Linux > > > > issue mentioned above: i32 codegen where i64 is expected. Of course I > > > > could improve the condition to handle Linux, but at that point I just > > > > thing it's better to use Darwin, which is what the fix is towards > > > > anyway. Additional ideas? > > > I don't think we should sweep this under the rug just because there are > > > some test failures. There is probably some latent bug worth investigating. > > I think I remember answer a question for Albert during his internship, and > > I said something like "they should stay longs", so he added those tests. > > Thinking about it now, those test should be changed to expect 32-bit ints. > Well, there's specific testing for this behavior under Linux, so I assume > someone needs this? I don't see how this is sweeping stuff under the rug. --- Oh, I see. Gonna rework those then! https://reviews.llvm.org/D34377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34377: Support MS builtins using 'long' on LP64
bruno updated this revision to Diff 103287.
bruno retitled this revision from "Support MS builtins using 'long' on
Darwin/LP64" to "Support MS builtins using 'long' on LP64".
bruno edited the summary of this revision.
bruno added a comment.
New patch addressing comments from reviewers.
https://reviews.llvm.org/D34377
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/BuiltinsARM.def
include/clang/Basic/BuiltinsX86.def
include/clang/Basic/BuiltinsX86_64.def
lib/AST/ASTContext.cpp
test/CodeGen/ms-intrinsics-darwin.c
test/CodeGen/ms-intrinsics-rotations.c
test/CodeGen/pr27892.c
Index: test/CodeGen/pr27892.c
===
--- test/CodeGen/pr27892.c
+++ test/CodeGen/pr27892.c
@@ -1,23 +1,30 @@
// RUN: %clang_cc1 -triple x86_64-linux-gnu -fms-extensions %s -emit-llvm -o - | FileCheck %s
-long test1(long *p) {
+// LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
+#ifdef __LP64__
+#define LONG int
+#else
+#define LONG long
+#endif
+
+LONG test1(LONG *p) {
return _InterlockedIncrement(p);
}
-// CHECK-DAG: define i64 @test1(
-// CHECK: %[[p_addr:.*]] = alloca i64*, align 8
-// CHECK: store i64* %p, i64** %[[p_addr]], align 8
-// CHECK: %[[p_load:.*]] = load i64*, i64** %[[p_addr]], align 8
-// CHECK: %[[atomic_add:.*]] = atomicrmw add i64* %[[p_load]], i64 1 seq_cst
-// CHECK: %[[res:.*]] = add i64 %[[atomic_add]], 1
-// CHECK: ret i64 %[[res]]
+// CHECK-DAG: define i32 @test1(
+// CHECK: %[[p_addr:.*]] = alloca i32*, align 8
+// CHECK: store i32* %p, i32** %[[p_addr]], align 8
+// CHECK: %[[p_load:.*]] = load i32*, i32** %[[p_addr]], align 8
+// CHECK: %[[atomic_add:.*]] = atomicrmw add i32* %[[p_load]], i32 1 seq_cst
+// CHECK: %[[res:.*]] = add i32 %[[atomic_add]], 1
+// CHECK: ret i32 %[[res]]
-long test2(long *p) {
+LONG test2(LONG *p) {
return _InterlockedDecrement(p);
}
-// CHECK-DAG: define i64 @test2(
-// CHECK: %[[p_addr:.*]] = alloca i64*, align 8
-// CHECK: store i64* %p, i64** %[[p_addr]], align 8
-// CHECK: %[[p_load:.*]] = load i64*, i64** %[[p_addr]], align 8
-// CHECK: %[[atomic_sub:.*]] = atomicrmw sub i64* %[[p_load]], i64 1 seq_cst
-// CHECK: %[[res:.*]] = sub i64 %[[atomic_sub]], 1
-// CHECK: ret i64 %[[res]]
+// CHECK-DAG: define i32 @test2(
+// CHECK: %[[p_addr:.*]] = alloca i32*, align 8
+// CHECK: store i32* %p, i32** %[[p_addr]], align 8
+// CHECK: %[[p_load:.*]] = load i32*, i32** %[[p_addr]], align 8
+// CHECK: %[[atomic_sub:.*]] = atomicrmw sub i32* %[[p_load]], i32 1 seq_cst
+// CHECK: %[[res:.*]] = sub i32 %[[atomic_sub]], 1
+// CHECK: ret i32 %[[res]]
Index: test/CodeGen/ms-intrinsics-rotations.c
===
--- test/CodeGen/ms-intrinsics-rotations.c
+++ test/CodeGen/ms-intrinsics-rotations.c
@@ -12,7 +12,17 @@
// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
// RUN: -triple x86_64--linux -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -fms-extensions \
+// RUN: -triple x86_64--darwin -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+
+// LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
+#ifdef __LP64__
+#define LONG int
+#else
+#define LONG long
+#endif
// rotate left
@@ -58,7 +68,7 @@
// CHECK: ret i32 [[RESULT]]
// CHECK }
-unsigned long test_lrotl(unsigned long value, int shift) {
+unsigned LONG test_lrotl(unsigned LONG value, int shift) {
return _lrotl(value, shift);
}
// CHECK-32BIT-LONG: i32 @test_lrotl
@@ -72,17 +82,6 @@
// CHECK-32BIT-LONG: ret i32 [[RESULT]]
// CHECK-32BIT-LONG }
-// CHECK-64BIT-LONG: i64 @test_lrotl
-// CHECK-64BIT-LONG: [[SHIFT:%[0-9]+]] = and i64 %{{[0-9]+}}, 63
-// CHECK-64BIT-LONG: [[NEGSHIFT:%[0-9]+]] = sub i64 64, [[SHIFT]]
-// CHECK-64BIT-LONG: [[HIGH:%[0-9]+]] = shl i64 [[VALUE:%[0-9]+]], [[SHIFT]]
-// CHECK-64BIT-LONG: [[LOW:%[0-9]+]] = lshr i64 [[VALUE]], [[NEGSHIFT]]
-// CHECK-64BIT-LONG: [[ROTATED:%[0-9]+]] = or i64 [[HIGH]], [[LOW]]
-// CHECK-64BIT-LONG: [[ISZERO:%[0-9]+]] = icmp eq i64 [[SHIFT]], 0
-// CHECK-64BIT-LONG: [[RESULT:%[0-9]+]] = select i1 [[ISZERO]], i64 [[VALUE]], i64 [[ROTATED]]
-// CHECK-64BIT-LONG: ret i64 [[RESULT]]
-// CHECK-64BIT-LONG }
-
unsigned __int64 test_rotl64(unsigned __int64 value, int shift) {
return _rotl64(value, shift);
}
@@ -141,7 +140,7 @@
// CHECK: ret i32 [[RESULT]]
// CHECK }
-unsigned long test_lrotr(unsigned long value, int shift) {
+unsigned LONG test_lrotr(unsigned LONG value, int shift) {
return _lrotr(value, shift);
}
// CHECK-32BIT-LONG: i32 @test_lrotr
@@ -155,17 +154,6
[PATCH] D34377: Support MS builtins using 'long' on LP64
bruno marked 3 inline comments as done. bruno added inline comments. Comment at: test/CodeGen/ms-intrinsics-darwin.c:1 +// RUN: %clang_cc1 -ffreestanding -fms-extensions \ +// RUN: -triple x86_64--darwin -Oz -emit-llvm %s -o - \ rnk wrote: > I'd rather generalize `test/CodeGen/ms-intrinsics.c` to cover Darwin as well > as Windows & Linux. ms-intrinsics.c currently includes intrin.h and does not cover linux. Instead I'm going to rename this ms-intrinsics-other.c, test for linux as well and cover the content from pr27892.c. https://reviews.llvm.org/D34377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34377: Support MS builtins using 'long' on LP64
bruno updated this revision to Diff 103315.
bruno marked an inline comment as done.
bruno added a comment.
Addressing comments!
https://reviews.llvm.org/D34377
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/BuiltinsARM.def
include/clang/Basic/BuiltinsX86.def
include/clang/Basic/BuiltinsX86_64.def
lib/AST/ASTContext.cpp
test/CodeGen/ms-intrinsics-other.c
test/CodeGen/ms-intrinsics-rotations.c
test/CodeGen/pr27892.c
Index: test/CodeGen/pr27892.c
===
--- test/CodeGen/pr27892.c
+++ /dev/null
@@ -1,23 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fms-extensions %s -emit-llvm -o - | FileCheck %s
-
-long test1(long *p) {
- return _InterlockedIncrement(p);
-}
-// CHECK-DAG: define i64 @test1(
-// CHECK: %[[p_addr:.*]] = alloca i64*, align 8
-// CHECK: store i64* %p, i64** %[[p_addr]], align 8
-// CHECK: %[[p_load:.*]] = load i64*, i64** %[[p_addr]], align 8
-// CHECK: %[[atomic_add:.*]] = atomicrmw add i64* %[[p_load]], i64 1 seq_cst
-// CHECK: %[[res:.*]] = add i64 %[[atomic_add]], 1
-// CHECK: ret i64 %[[res]]
-
-long test2(long *p) {
- return _InterlockedDecrement(p);
-}
-// CHECK-DAG: define i64 @test2(
-// CHECK: %[[p_addr:.*]] = alloca i64*, align 8
-// CHECK: store i64* %p, i64** %[[p_addr]], align 8
-// CHECK: %[[p_load:.*]] = load i64*, i64** %[[p_addr]], align 8
-// CHECK: %[[atomic_sub:.*]] = atomicrmw sub i64* %[[p_load]], i64 1 seq_cst
-// CHECK: %[[res:.*]] = sub i64 %[[atomic_sub]], 1
-// CHECK: ret i64 %[[res]]
Index: test/CodeGen/ms-intrinsics-rotations.c
===
--- test/CodeGen/ms-intrinsics-rotations.c
+++ test/CodeGen/ms-intrinsics-rotations.c
@@ -12,7 +12,17 @@
// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
// RUN: -triple x86_64--linux -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -fms-extensions \
+// RUN: -triple x86_64--darwin -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+
+// LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
+#ifdef __LP64__
+#define LONG int
+#else
+#define LONG long
+#endif
// rotate left
@@ -58,7 +68,7 @@
// CHECK: ret i32 [[RESULT]]
// CHECK }
-unsigned long test_lrotl(unsigned long value, int shift) {
+unsigned LONG test_lrotl(unsigned LONG value, int shift) {
return _lrotl(value, shift);
}
// CHECK-32BIT-LONG: i32 @test_lrotl
@@ -72,17 +82,6 @@
// CHECK-32BIT-LONG: ret i32 [[RESULT]]
// CHECK-32BIT-LONG }
-// CHECK-64BIT-LONG: i64 @test_lrotl
-// CHECK-64BIT-LONG: [[SHIFT:%[0-9]+]] = and i64 %{{[0-9]+}}, 63
-// CHECK-64BIT-LONG: [[NEGSHIFT:%[0-9]+]] = sub i64 64, [[SHIFT]]
-// CHECK-64BIT-LONG: [[HIGH:%[0-9]+]] = shl i64 [[VALUE:%[0-9]+]], [[SHIFT]]
-// CHECK-64BIT-LONG: [[LOW:%[0-9]+]] = lshr i64 [[VALUE]], [[NEGSHIFT]]
-// CHECK-64BIT-LONG: [[ROTATED:%[0-9]+]] = or i64 [[HIGH]], [[LOW]]
-// CHECK-64BIT-LONG: [[ISZERO:%[0-9]+]] = icmp eq i64 [[SHIFT]], 0
-// CHECK-64BIT-LONG: [[RESULT:%[0-9]+]] = select i1 [[ISZERO]], i64 [[VALUE]], i64 [[ROTATED]]
-// CHECK-64BIT-LONG: ret i64 [[RESULT]]
-// CHECK-64BIT-LONG }
-
unsigned __int64 test_rotl64(unsigned __int64 value, int shift) {
return _rotl64(value, shift);
}
@@ -141,7 +140,7 @@
// CHECK: ret i32 [[RESULT]]
// CHECK }
-unsigned long test_lrotr(unsigned long value, int shift) {
+unsigned LONG test_lrotr(unsigned LONG value, int shift) {
return _lrotr(value, shift);
}
// CHECK-32BIT-LONG: i32 @test_lrotr
@@ -155,17 +154,6 @@
// CHECK-32BIT-LONG: ret i32 [[RESULT]]
// CHECK-32BIT-LONG }
-// CHECK-64BIT-LONG: i64 @test_lrotr
-// CHECK-64BIT-LONG: [[SHIFT:%[0-9]+]] = and i64 %{{[0-9]+}}, 63
-// CHECK-64BIT-LONG: [[NEGSHIFT:%[0-9]+]] = sub i64 64, [[SHIFT]]
-// CHECK-64BIT-LONG: [[LOW:%[0-9]+]] = lshr i64 [[VALUE:%[0-9]+]], [[SHIFT]]
-// CHECK-64BIT-LONG: [[HIGH:%[0-9]+]] = shl i64 [[VALUE]], [[NEGSHIFT]]
-// CHECK-64BIT-LONG: [[ROTATED:%[0-9]+]] = or i64 [[HIGH]], [[LOW]]
-// CHECK-64BIT-LONG: [[ISZERO:%[0-9]+]] = icmp eq i64 [[SHIFT]], 0
-// CHECK-64BIT-LONG: [[RESULT:%[0-9]+]] = select i1 [[ISZERO]], i64 [[VALUE]], i64 [[ROTATED]]
-// CHECK-64BIT-LONG: ret i64 [[RESULT]]
-// CHECK-64BIT-LONG }
-
unsigned __int64 test_rotr64(unsigned __int64 value, int shift) {
return _rotr64(value, shift);
}
Index: test/CodeGen/ms-intrinsics-other.c
===
--- /dev/null
+++ test/CodeGen/ms-intrinsics-other.c
@@ -0,0 +1,161 @@
+// RUN: %clang_cc1 -ffreestanding -fms-extensions \
+// RUN: -triple x86_64--darwin -Oz -emit-llvm %s -o -
[PATCH] D34377: Support MS builtins using 'long' on LP64
This revision was automatically updated to reflect the committed changes. Closed by commit rL305875: Support MS builtins using 'long' on LP64 platforms (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D34377?vs=103315&id=103318#toc Repository: rL LLVM https://reviews.llvm.org/D34377 Files: cfe/trunk/include/clang/Basic/Builtins.def cfe/trunk/include/clang/Basic/BuiltinsARM.def cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/include/clang/Basic/BuiltinsX86_64.def cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/test/CodeGen/ms-intrinsics-other.c cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c cfe/trunk/test/CodeGen/pr27892.c Index: cfe/trunk/include/clang/Basic/BuiltinsARM.def === --- cfe/trunk/include/clang/Basic/BuiltinsARM.def +++ cfe/trunk/include/clang/Basic/BuiltinsARM.def @@ -215,10 +215,10 @@ LANGBUILTIN(_MoveToCoprocessor, "vUiIUiIUiIUiIUiIUi", "", ALL_MS_LANGUAGES) LANGBUILTIN(_MoveToCoprocessor2, "vUiIUiIUiIUiIUiIUi", "", ALL_MS_LANGUAGES) -TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(_BitScanForward64, "UcULi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcULi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") TARGET_HEADER_BUILTIN(_InterlockedDecrement64, "LLiLLiD*","nh", "intrin.h", ALL_MS_LANGUAGES, "") Index: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def === --- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def +++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def @@ -22,8 +22,8 @@ # define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) BUILTIN(ID, TYPE, ATTRS) #endif -TARGET_HEADER_BUILTIN(_BitScanForward64, "UcULi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcULi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") TARGET_HEADER_BUILTIN(__mulh, "LLiLLiLLi","nch", "intrin.h", ALL_MS_LANGUAGES, "") TARGET_HEADER_BUILTIN(__umulh, "ULLiULLiULLi", "nch", "intrin.h", ALL_MS_LANGUAGES, "") Index: cfe/trunk/include/clang/Basic/BuiltinsX86.def === --- cfe/trunk/include/clang/Basic/BuiltinsX86.def +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def @@ -1822,8 +1822,8 @@ TARGET_BUILTIN(__builtin_ia32_clzero, "vv*", "", "clzero") // MSVC -TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", ALL_MS_LANGUAGES, "") TARGET_HEADER_BUILTIN(_ReadBarrier, "v", "nh", "intrin.h", ALL_MS_LANGUAGES, "") @@ -1838,15 +1838,15 @@ TARGET_HEADER_BUILTIN(__int2c, "v", "nr", "intrin.h", ALL_MS_LANGUAGES, "") TARGET_HEADER_BUILTIN(__ud2, "v", "nr", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(__readfsbyte, "UcULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(__readfsword, "UsULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(__readfsdword, "ULiULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(__readfsqword, "ULLiULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") - -TARGET_HEADER_BUILTIN(__readgsbyte, "UcULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(__readgsword, "UsULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(__readgsdword, "ULiULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") -TARGET_HEADER_BUILTIN(__readgsqword, "ULLiULi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(__readfsbyte, "UcUNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(__readfsword, "UsUNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(__readfsdword, "UNiUNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_
[PATCH] D34469: Use vfs::FileSystem in ASTUnit when creating CompilerInvocation.
bruno added a reviewer: bruno. bruno added a comment. Any reason why this doesn't contain a testcase? https://reviews.llvm.org/D34469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT
bruno created this revision. _LIBCPP_MSVCRT is defined because _WIN32 is defined and __MINGW32__ is not. Some non-windows targets using MS extensions define _WIN32 for compatibility with Windows but do not have MSVC compatibility. This patch is an attempt to do not have _LIBCPP_MSVCRT defined for such targets, allowing libcxx to build. This patch seems the natural way to go for me, but others more experienced here might have additional suggestions? https://reviews.llvm.org/D34588 Files: include/__config Index: include/__config === --- include/__config +++ include/__config @@ -229,9 +229,12 @@ # define _LIBCPP_SHORT_WCHAR 1 // Both MinGW and native MSVC provide a "MSVC"-like enviroment # define _LIBCPP_MSVCRT_LIKE -// If mingw not explicitly detected, assume using MS C runtime only. +// If mingw not explicitly detected, assume using MS C runtime only if +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#endif # endif # if (defined(_M_AMD64) || defined(__x86_64__)) || (defined(_M_ARM) || defined(__arm__)) #define _LIBCPP_HAS_BITSCAN64 Index: include/__config === --- include/__config +++ include/__config @@ -229,9 +229,12 @@ # define _LIBCPP_SHORT_WCHAR 1 // Both MinGW and native MSVC provide a "MSVC"-like enviroment # define _LIBCPP_MSVCRT_LIKE -// If mingw not explicitly detected, assume using MS C runtime only. +// If mingw not explicitly detected, assume using MS C runtime only if +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#endif # endif # if (defined(_M_AMD64) || defined(__x86_64__)) || (defined(_M_ARM) || defined(__arm__)) #define _LIBCPP_HAS_BITSCAN64 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT
bruno marked an inline comment as done. bruno added inline comments. Comment at: include/__config:234-235 +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library bcraig wrote: > majnemer wrote: > > compnerd wrote: > > > smeenai wrote: > > > > You can combine this into just > > > > > > > > ``` > > > > # if defined(_MSC_VER) && !defined(__MINGW32__) > > > > ``` > > > > > > > > I don't know if `__MINGW32__` and `_MSC_VER` will ever be compiled > > > > simultaneously. (clang never defines `_MSC_VER` for its MinGW triples, > > > > for example.) > > > What if MinGW is built with clang/c2 and MSVC extensions? I think that > > > the two could be defined together. What about cygwin and clang/c2? I > > > guess we can ignore that since cygwin is not under active development. > > > > > > I think this really goes back to my idea for an additional flag to > > > indicate the C library in use. We can interpret it from the > > > canonicalized triple that LLVM/clang use. > > clang/c2 is dead. > At some point, I would like to see (or will need to introduce) a flag for > which Windows C library is in use (so I'm agreeing with / echoing @compnerd). > What all options are there right now? There's the Visual Studio C-runtime > (multiple versions), there's msvcrt (used by the OS and mingw), there's the > ancient crtdll that we shouldn't ever support, and there's the kernel C > runtime (I'm probably the only person that cares about that). > > I will note that I don't like the name of the macro here. _LIBCPP_MSVCRT > implies that msvcrt.dll is being used, when it isn't. I don't think that > this patch needs to fix that naming though. Any suggestion on a new name instead of `_LIBCPP_MSVCRT` for a future patch? https://reviews.llvm.org/D34588 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT
bruno updated this revision to Diff 103975. bruno added a comment. Update patch after reviewer suggestions! https://reviews.llvm.org/D34588 Files: include/__config Index: include/__config === --- include/__config +++ include/__config @@ -229,8 +229,9 @@ # define _LIBCPP_SHORT_WCHAR 1 // Both MinGW and native MSVC provide a "MSVC"-like enviroment # define _LIBCPP_MSVCRT_LIKE -// If mingw not explicitly detected, assume using MS C runtime only. -# ifndef __MINGW32__ +// If mingw not explicitly detected, assume using MS C runtime only if +// a MS compatibility version is specified. +# if defined(_MSC_VER) && !defined(__MINGW32__) #define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library # endif # if (defined(_M_AMD64) || defined(__x86_64__)) || (defined(_M_ARM) || defined(__arm__)) Index: include/__config === --- include/__config +++ include/__config @@ -229,8 +229,9 @@ # define _LIBCPP_SHORT_WCHAR 1 // Both MinGW and native MSVC provide a "MSVC"-like enviroment # define _LIBCPP_MSVCRT_LIKE -// If mingw not explicitly detected, assume using MS C runtime only. -# ifndef __MINGW32__ +// If mingw not explicitly detected, assume using MS C runtime only if +// a MS compatibility version is specified. +# if defined(_MSC_VER) && !defined(__MINGW32__) #define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library # endif # if (defined(_M_AMD64) || defined(__x86_64__)) || (defined(_M_ARM) || defined(__arm__)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40983: Generate Libclang invocation reproducers using a new -cc1gen-reproducer option
bruno added a comment.
Hi Alex,
Thanks for improving this.
- Instead of adding `-cc1gen-reproducer`, why can't you run that through `-cc1`
and have a flag similar to `-###`, which just prints the reproducer line?
- I didn't understand how you can use the final output information, can you
give an example to illustrate?
Comment at: include/clang/Driver/Driver.h:394
+ struct CompilationDiagnosticReport {
+std::vector TemporaryFiles;
+ };
I assume the number of temporary files are usually small, can you switch to
SmallVector here?
Comment at: tools/driver/cc1gen_reproducer_main.cpp:187
+ // Emit the information about the reproduce files to stdout.
+ int Result;
+ if (Report) {
int Result = 1;
(no need for the else clause)
Repository:
rC Clang
https://reviews.llvm.org/D40983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41545: Replace cp -a in various Clang tests
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Hi Hubert, Thanks for fixing this. `cp -R` is sufficient IMO. LGTM Repository: rC Clang https://reviews.llvm.org/D41545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31269: [Modules] Allow modules specified by -fmodule-map-file to shadow implicitly found ones
bruno added a comment. > It might make more sense to have the module loaded from the AST file shadow > the module from the module map, especially for an explicit module build, now > that we have that functionality.) +1, seems a much more consistent model. Comment at: lib/Lex/ModuleMap.cpp:2574-2575 + llvm::SaveAndRestore OldExplicit(CurrentModuleMapIsExplicitlyProvided); + CurrentModuleMapIsExplicitlyProvided |= IsExplicitlyProvided; + rsmith wrote: > It would seem cleaner to make this a member of `ModuleMapParser` (and > explicitly pass down the flag when parsing an `extern module` declaration). > Is there a reason to use (essentially) global state for this? I don't believe there's any reason for using a global state here (and Ben doesn't recall any specific reason either). I changed the patch to pass down the flag and it works fine. https://reviews.llvm.org/D31269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31269: [Modules] Allow modules specified by -fmodule-map-file to shadow implicitly found ones
This revision was automatically updated to reflect the committed changes.
Closed by commit rL321781: [Modules] Allow modules specified by
-fmodule-map-file to shadow implicitly… (authored by bruno, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D31269?vs=92738&id=128590#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31269
Files:
cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
cfe/trunk/include/clang/Basic/Module.h
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/lib/Basic/Module.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Modules/Inputs/shadow/A1/A.h
cfe/trunk/test/Modules/Inputs/shadow/A1/module.modulemap
cfe/trunk/test/Modules/Inputs/shadow/A2/A.h
cfe/trunk/test/Modules/Inputs/shadow/A2/module.modulemap
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A1/Foo.h
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A1/module.modulemap
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A1/sys/A.h
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A1/sys/A2.h
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A2/Foo.h
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A2/module.modulemap
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A2/sys/A.h
cfe/trunk/test/Modules/Inputs/shadowed-submodule/A2/sys/A2.h
cfe/trunk/test/Modules/Inputs/shadowed-submodule/Foo/module.modulemap
cfe/trunk/test/Modules/shadow.m
cfe/trunk/test/Modules/shadowed-submodule.m
Index: cfe/trunk/include/clang/Lex/ModuleMap.h
===
--- cfe/trunk/include/clang/Lex/ModuleMap.h
+++ cfe/trunk/include/clang/Lex/ModuleMap.h
@@ -195,6 +195,17 @@
/// header.
llvm::DenseMap UmbrellaDirs;
+ /// \brief The set of modules provided explicitly (e.g. by -fmodule-map-file),
+ /// which are allowed to shadow other implicitly discovered modules.
+ llvm::DenseSet ExplicitlyProvidedModules;
+
+ bool mayShadowModuleBeingParsed(Module *ExistingModule,
+ bool IsExplicitlyProvided) {
+assert(!ExistingModule->Parent && "expected top-level module");
+return !IsExplicitlyProvided &&
+ ExplicitlyProvidedModules.count(ExistingModule);
+ }
+
/// \brief The set of attributes that can be attached to a module.
struct Attributes {
/// \brief Whether this is a system module.
@@ -475,9 +486,9 @@
///
/// \returns The found or newly-created module, along with a boolean value
/// that will be true if the module is newly-created.
- std::pair findOrCreateModule(StringRef Name, Module *Parent,
- bool IsFramework,
- bool IsExplicit);
+ std::pair
+ findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
+ bool IsExplicit, bool UsesExplicitModuleMapFile = false);
/// \brief Create a 'global module' for a C++ Modules TS module interface
/// unit.
@@ -502,6 +513,11 @@
Module *inferFrameworkModule(const DirectoryEntry *FrameworkDir,
bool IsSystem, Module *Parent);
+ /// \brief Create a new top-level module that is shadowed by
+ /// \p ShadowingModule.
+ Module *createShadowedModule(StringRef Name, bool IsFramework,
+ Module *ShadowingModule);
+
/// \brief Retrieve the module map file containing the definition of the given
/// module.
///
@@ -587,6 +603,8 @@
/// \brief Marks this header as being excluded from the given module.
void excludeHeader(Module *Mod, Module::Header Header);
+ void setExplicitlyProvided(Module *Mod);
+
/// \brief Parse the given module map file, and record any modules we
/// encounter.
///
@@ -606,10 +624,15 @@
/// \param ExternModuleLoc The location of the "extern module" declaration
///that caused us to load this module map file, if any.
///
+ /// \param IsExplicitlyProvided Whether this module map file was provided
+ /// explicitly by the user (e.g. -fmodule-map-file), rather than found
+ /// implicitly.
+ ///
/// \returns true if an error occurred, false otherwise.
bool parseModuleMapFile(const FileEntry *File, bool IsSystem,
- const DirectoryEntry *HomeDir, FileID ID = FileID(),
- unsigned *Offset = nullptr,
+ const DirectoryEntry *HomeDir,
+ bool IsExplicitlyProvided = false,
+ FileID ID = FileID(), unsigned *Offset = nullptr,
SourceLocation ExternModuleLoc = SourceLocation());
/// \brief Dump the contents of the module map, for debugging purposes.
Index: cfe/trunk/include/clang/Lex/HeaderSearch.h
===
--- cfe/trunk/include/clang/Lex/HeaderSearch.h
+++
[PATCH] D40712: [Driver] Add flag enabling the function stack size section that was added in r319430
bruno added inline comments.
Comment at: lib/Driver/ToolChains/Clang.cpp:3783
+CmdArgs.push_back("-fno-stack-size-section");
+
CmdArgs.push_back("-ferror-limit");
What happens when you invoke cc1 directly for ps4 and don't specify any of the
two options? Is it going to default to not using stack size section? It also
seems that all non ps4 targets will get -fno-stack-size-section flag by default
in cc1, is it really needed?
https://reviews.llvm.org/D40712
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41544: Use backslash escape, replacing xargs -0 in test macro-multiline.c
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D41544 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41733: [Driver] Suggest correctly spelled driver options
bruno added a comment. This is great! I assume it also works for cc1 invocations, right? Can you also add a test for %clang_cc1? Repository: rC Clang https://reviews.llvm.org/D41733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
bruno added a comment. Herald added a subscriber: mgrang. The change seems good to me in general. I wonder if this will hit any broken assumption in the code. Did you run other tests beside unittests? https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41733: [Driver] Suggest correctly spelled driver options
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks! LGTM. Repository: rC Clang https://reviews.llvm.org/D41733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40712: [Driver] Add flag enabling the function stack size section that was added in r319430
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks! LGTM https://reviews.llvm.org/D40712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40983: Generate Libclang invocation reproducers using a new -cc1gen-reproducer option
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Makes sense, LGTM. Should we add documentation explaining how to use this? I'm fine if it comes in a follow up commit. https://reviews.llvm.org/D40983 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47157: Warning for framework headers using double quote includes
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47157: Warning for framework headers using double quote includes
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 include" + >, InGroup, 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 > > > `, 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 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
[PATCH] D47157: Warning for framework headers using double quote includes
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 include" + >, InGroup, DefaultIgnore; dexonsmith wrote: > bruno wrote: > > 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 `, 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 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). > Can we just check if it's a header in the *same* framework? For some pretty obvious cases we can probably assume that this is what the user wants, but even so it might be misleading. For example, if you're in `ABC/H1.h`, you include `H2.h` and the framework ABC has an `ABC/H2.h`. It could be that `#include "H2.h"` is mapped via header maps to `$SOURCE/H2.h` instead of using the installed headers in the framework style build products. This is likely a mistake, but what if it's intentional? I would prefer if the user rethink it instead of just buying potential misleading clues. OTOH, I share the concern that we don't need to be perfect here and only emit the fixit for really obvious cases, and not for the others. Will update the patch to include a fixit to the very straightforward scenario: `H2.h` was found in the same framework style path as the includer. 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
[PATCH] D47157: Warning for framework headers using double quote includes
bruno updated this revision to Diff 149313.
bruno added a comment.
Updated the patch after Duncan and Aaron reviews. I actually went a bit more
aggressive with the fixits, since I realized the conditions for the warning are
already strict enough and we should take the chance to be more clear. For the
attached testcase, the output now is:
./A.framework/Headers/A.h:2:10: warning: double-quoted include "A0.h" in
framework header, expected angle-bracketed include instead
#include "A0.h"
^~
./A.framework/Headers/A.h:3:10: warning: double-quoted include "B.h" in
framework header, expected angle-bracketed include instead
#include "B.h"
^
https://reviews.llvm.org/D47157
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/HeaderSearch.cpp
test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
test/Modules/Inputs/double-quotes/B.h
test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
test/Modules/Inputs/double-quotes/a.hmap.json
test/Modules/Inputs/double-quotes/flat-header-path/Z.h
test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
test/Modules/Inputs/double-quotes/x.hmap.json
test/Modules/Inputs/double-quotes/z.yaml
test/Modules/double-quotes.m
Index: test/Modules/double-quotes.m
===
--- /dev/null
+++ test/Modules/double-quotes.m
@@ -0,0 +1,38 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/double-quotes/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/double-quotes/x.hmap.json %t/x.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/double-quotes:g" \
+// RUN: %S/Inputs/double-quotes/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
+// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
+// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s \
+// RUN: 2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed include instead
+// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead
+// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead
+
+#import "A.h"
+#import
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:2{{double-quoted include "A0.h" in framework header}}
+// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:3{{double-quoted include "B.h" in framework header}}
+// expected-warning@Inputs/double-quotes/flat-header-path/Z.h:1{{double-quoted include "B.h" in framework header}}
Index: test/Modules/Inputs/double-quotes/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/z.yaml
@@ -0,0 +1,28 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Modules",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+}
+ ]
+}
+ ]
+}
Index: test/Modules/Inputs/double-quotes/x.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/x.hmap.json
@@ -0,0 +1,7 @@
+
+{
+ "mappings" :
+{
+ "X.h" : "X/X.h"
+}
+}
Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
@@ -0,0 +1,3 @@
+framework module Z {
+ header "Z.h"
+}
Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.h
===
[PATCH] D47157: Warning for framework headers using double quote includes
bruno updated this revision to Diff 149359.
bruno edited the summary of this revision.
bruno added a comment.
Update after Duncan's review: remove header name from the warning message
(since it's already in the fixit)
https://reviews.llvm.org/D47157
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/HeaderSearch.cpp
test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
test/Modules/Inputs/double-quotes/B.h
test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
test/Modules/Inputs/double-quotes/a.hmap.json
test/Modules/Inputs/double-quotes/flat-header-path/Z.h
test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
test/Modules/Inputs/double-quotes/x.hmap.json
test/Modules/Inputs/double-quotes/z.yaml
test/Modules/double-quotes.m
Index: test/Modules/double-quotes.m
===
--- /dev/null
+++ test/Modules/double-quotes.m
@@ -0,0 +1,39 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/double-quotes/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/double-quotes/x.hmap.json %t/x.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/double-quotes:g" \
+// RUN: %S/Inputs/double-quotes/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same
+
+// RUN: %clang_cc1 \
+// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
+// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
+// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s \
+// RUN: 2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed instead
+// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+
+#import "A.h"
+#import
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted include "A0.h" in framework header, expected angle-bracketed instead}}
+// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:2{{double-quoted include "B.h" in framework header, expected angle-bracketed instead}}
+// expected-warning@Inputs/double-quotes/flat-header-path/Z.h:1{{double-quoted include "B.h" in framework header, expected angle-bracketed instead}}
Index: test/Modules/Inputs/double-quotes/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/z.yaml
@@ -0,0 +1,28 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Modules",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+}
+ ]
+}
+ ]
+}
Index: test/Modules/Inputs/double-quotes/x.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/x.hmap.json
@@ -0,0 +1,7 @@
+
+{
+ "mappings" :
+{
+ "X.h" : "X/X.h"
+}
+}
Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
@@ -0,0 +1,3 @@
+framework module Z {
+ header "Z.h"
+}
Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.h
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/flat-header-path/Z.h
@@ -0,0 +1 @@
+#import "B.h" // Included from Z.h & A.h
Index: test/Modules/Inputs/double-quotes/a.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/double-quotes/a.hmap.json
@@ -0,0 +1,7 @@
+
+{
+ "mappings" :
+{
+ "A.h" : "A/A.h"
+}
+}
Index: test/Modules/Inputs/double-quot
[PATCH] D47157: Warning for framework headers using double quote includes
bruno added inline comments. Comment at: test/Modules/double-quotes.m:24-25 + +// The same warnings show up when modules is on but -verify doesn't get it +// because they only show up under the module A building context. +// RUN: FileCheck --input-file=%t/stderr %s dexonsmith wrote: > Would using an explicit module build make this any easier? --- Not really =T https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno updated this revision to Diff 149361.
bruno added a comment.
Update patch after changes to https://reviews.llvm.org/D47157. Also address
some of Volodymyr feedback.
https://reviews.llvm.org/D47301
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/HeaderSearch.cpp
test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
test/Modules/Inputs/framework-public-includes-private/a.hmap.json
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
test/Modules/Inputs/framework-public-includes-private/z.hmap.json
test/Modules/Inputs/framework-public-includes-private/z.yaml
test/Modules/framework-public-includes-private.m
Index: test/Modules/framework-public-includes-private.m
===
--- /dev/null
+++ test/Modules/framework-public-includes-private.m
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \
+// RUN: %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s \
+// RUN: 2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: public framework header includes private framework header 'A/APriv.h'
+// CHECK: public framework header includes private framework header 'A/APriv2.h'
+// CHECK: public framework header includes private framework header 'Z/ZPriv.h'
+
+#import "A.h"
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv.h'}}
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}}
+// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}}
Index: test/Modules/Inputs/framework-public-includes-private/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.yaml
@@ -0,0 +1,45 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'use-external-names' : 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/PrivateHeaders",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "ZPriv.h",
+ 'external-contents': "TEST_DIR/flat-header-path/ZPriv.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Modules",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+},
+{
+ 'type': 'file',
+ 'name': "module.private.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.private.modulemap"
+}
+ ]
+}
+ ]
+}
Index: test/Modules/Inputs/framework-public-includes-private/z.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/fr
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno marked an inline comment as done.
bruno added inline comments.
Comment at: include/clang/Basic/DiagnosticGroups.td:34-35
def AutoImport : DiagGroup<"auto-import">;
def FrameworkHdrQuotedInclude :
DiagGroup<"quoted-include-in-framework-header">;
+def FrameworkIncludePrivateFromPublic :
DiagGroup<"framework-include-private-from-public">;
def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">;
vsapsai wrote:
> It might be convenient for users to have a warning group that will cover
> different framework warnings, something like -Wframework-hygiene. But it's
> out of scope for this review, more as an idea for future improvements.
Yep, that should come once we land all other bits.
Comment at: lib/Lex/HeaderSearch.cpp:625
+static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader,
+ SmallVectorImpl &FrameworkName) {
using namespace llvm::sys;
vsapsai wrote:
> In this function we accept some paths that aren't valid framework paths. Need
> to think more if it is OK or if we want to be stricter.
It should be ok at this point, otherwise the framework style path would have
failed before finding this header.
Comment at: test/Modules/framework-public-includes-private.m:33
+
+int bar() { return foo(); }
+
vsapsai wrote:
> I'm not entirely sure it's not covered by existing test. It might be worth
> testing private header including public header and private header including
> another private header.
The warning is on by default and clang already have the scenario you described
in other module tests - those would fail if there's a bug in the logic here.
https://reviews.llvm.org/D47301
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno added a comment. Ping. Without this patch, we would have to add binary header maps for tests in https://reviews.llvm.org/D47157 and https://reviews.llvm.org/D47301, which I would like to avoid. https://reviews.llvm.org/D46485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48297: [Darwin] Add a warning for missing include path for libstdc++
bruno added inline comments.
Comment at: lib/Frontend/InitHeaderSearch.cpp:491
} else {
- AddDefaultCPlusPlusIncludePaths(triple, HSOpts);
+ AddDefaultCPlusPlusIncludePaths(Lang, triple, HSOpts);
}
If we happen to suggest libc++, but it's also not there, do we still want the
warning? Should we check it exists first?
Repository:
rC Clang
https://reviews.llvm.org/D48297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48297: [Darwin] Add a warning for missing include path for libstdc++
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D48297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48367: [modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts
bruno added a comment. Hi Yuka, thanks for working on this. Comment at: clang/lib/Lex/HeaderSearch.cpp:285 // directory. -loadSubdirectoryModuleMaps(SearchDirs[Idx]); +if (ModMap.getLangOpts().ObjC1 || ModMap.getLangOpts().ObjC2) + loadSubdirectoryModuleMaps(SearchDirs[Idx]); aprantl wrote: > Are these flags also enabled in Objective-C++ mode? Looks like all this logic was introduced in r177621 to allow the names of modules to differ from the name of their subdirectory in the include path. Instead of having this to be based on the language, it's probably better if we have it based on @import name lookup, which is the scenario where we actually currently look more aggressively, did you try that path? This is also lacking a testcase, can you create one? https://reviews.llvm.org/D48367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46485: Add python tool to dump and construct header maps
bruno updated this revision to Diff 152142.
bruno added a comment.
Update after Richard's review, fix python3 compatibility and check for zero
sized string table
https://reviews.llvm.org/D46485
Files:
CMakeLists.txt
test/CMakeLists.txt
test/Modules/crash-vfs-headermaps.m
test/Preprocessor/Inputs/headermap-rel/foo.hmap
test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap.json
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
test/Preprocessor/headermap-rel.c
test/Preprocessor/headermap-rel2.c
test/Preprocessor/nonportable-include-with-hmap.c
utils/hmaptool/CMakeLists.txt
utils/hmaptool/hmaptool
Index: utils/hmaptool/hmaptool
===
--- /dev/null
+++ utils/hmaptool/hmaptool
@@ -0,0 +1,294 @@
+#!/usr/bin/env python
+from __future__ import print_function
+
+import json
+import optparse
+import os
+import struct
+import sys
+
+###
+
+k_header_magic_LE = 'pamh'
+k_header_magic_BE = 'hmap'
+
+def hmap_hash(str):
+"""hash(str) -> int
+
+Apply the "well-known" headermap hash function.
+"""
+
+return sum((ord(c.lower()) * 13
+for c in str), 0)
+
+class HeaderMap(object):
+@staticmethod
+def frompath(path):
+with open(path, 'rb') as f:
+magic = f.read(4)
+if magic == k_header_magic_LE:
+endian_code = '<'
+elif magic == k_header_magic_BE:
+endian_code = '>'
+else:
+raise SystemExit("error: %s: not a headermap" % (
+path,))
+
+# Read the header information.
+header_fmt = endian_code + 'HH'
+header_size = struct.calcsize(header_fmt)
+data = f.read(header_size)
+if len(data) != header_size:
+raise SystemExit("error: %s: truncated headermap header" % (
+path,))
+
+(version, reserved, strtable_offset, num_entries,
+ num_buckets, max_value_len) = struct.unpack(header_fmt, data)
+
+if version != 1:
+raise SystemExit("error: %s: unknown headermap version: %r" % (
+path, version))
+if reserved != 0:
+raise SystemExit("error: %s: invalid reserved value in header" % (
+path,))
+
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
+path,))
+
+# Read all of the buckets.
+bucket_fmt = endian_code + 'III'
+bucket_size = struct.calcsize(bucket_fmt)
+buckets_data = f.read(num_buckets * bucket_size)
+if len(buckets_data) != num_buckets * bucket_size:
+raise SystemExit("error: %s: truncated headermap buckets" % (
+path,))
+buckets = [struct.unpack(bucket_fmt,
+ buckets_data[i*bucket_size:(i+1)*bucket_size])
+ for i in range(num_buckets)]
+
+# Read the string table; the format doesn't explicitly communicate the
+# size of the string table (which is dumb), so assume it is the rest of
+# the file.
+f.seek(0, 2)
+strtable_size = f.tell() - strtable_offset
+f.seek(strtable_offset)
+
+if strtable_size == 0:
+raise SystemExit("error: %s: unable to read zero-sized string table"%(
+path,))
+strtable = f.read(strtable_size)
+
+if len(strtable) != strtable_size:
+raise SystemExit("error: %s: unable to read complete string table"%(
+path,))
+if strtable[-1] != '\0':
+raise SystemExit("error: %s: invalid string table in headermap" % (
+path,))
+
+return HeaderMap(num_entries, buckets, strtable)
+
+def __init__(self, num_entries, buckets, strtable):
+self.num_entries = num_entries
+self.buckets = buckets
+self.strtable = strtable
+
+def get_string(self, idx):
+if idx >= len(self.strtable):
+raise SystemExit("error: %s: invalid string index" % (
+path,))
+end_idx = self.strtable.index('\0', idx)
+return self.strtable[idx:end_idx]
+
+@property
+def mappings(self):
+for key_idx,prefix_idx,suffix_idx in self.buckets:
+if key_idx == 0:
+continue
+yield (self.get_string(key_idx),
+ self.get_string(prefix_idx) + self.get_string(suffix_idx))
[PATCH] D46485: Add python tool to dump and construct header maps
This revision was automatically updated to reflect the committed changes.
Closed by commit rC335177: Add python tool to dump and construct header maps
(authored by bruno, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46485?vs=152142&id=152168#toc
Repository:
rC Clang
https://reviews.llvm.org/D46485
Files:
CMakeLists.txt
test/CMakeLists.txt
test/Modules/crash-vfs-headermaps.m
test/Preprocessor/Inputs/headermap-rel/foo.hmap
test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap
test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap.json
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
test/Preprocessor/headermap-rel.c
test/Preprocessor/headermap-rel2.c
test/Preprocessor/nonportable-include-with-hmap.c
utils/hmaptool/CMakeLists.txt
utils/hmaptool/hmaptool
Index: utils/hmaptool/hmaptool
===
--- utils/hmaptool/hmaptool
+++ utils/hmaptool/hmaptool
@@ -0,0 +1,296 @@
+#!/usr/bin/env python
+from __future__ import print_function
+
+import json
+import optparse
+import os
+import struct
+import sys
+
+###
+
+k_header_magic_LE = 'pamh'
+k_header_magic_BE = 'hmap'
+
+def hmap_hash(str):
+"""hash(str) -> int
+
+Apply the "well-known" headermap hash function.
+"""
+
+return sum((ord(c.lower()) * 13
+for c in str), 0)
+
+class HeaderMap(object):
+@staticmethod
+def frompath(path):
+with open(path, 'rb') as f:
+magic = f.read(4)
+if magic == k_header_magic_LE:
+endian_code = '<'
+elif magic == k_header_magic_BE:
+endian_code = '>'
+else:
+raise SystemExit("error: %s: not a headermap" % (
+path,))
+
+# Read the header information.
+header_fmt = endian_code + 'HH'
+header_size = struct.calcsize(header_fmt)
+data = f.read(header_size)
+if len(data) != header_size:
+raise SystemExit("error: %s: truncated headermap header" % (
+path,))
+
+(version, reserved, strtable_offset, num_entries,
+ num_buckets, max_value_len) = struct.unpack(header_fmt, data)
+
+if version != 1:
+raise SystemExit("error: %s: unknown headermap version: %r" % (
+path, version))
+if reserved != 0:
+raise SystemExit("error: %s: invalid reserved value in header" % (
+path,))
+
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
+path,))
+
+# Read all of the buckets.
+bucket_fmt = endian_code + 'III'
+bucket_size = struct.calcsize(bucket_fmt)
+buckets_data = f.read(num_buckets * bucket_size)
+if len(buckets_data) != num_buckets * bucket_size:
+raise SystemExit("error: %s: truncated headermap buckets" % (
+path,))
+buckets = [struct.unpack(bucket_fmt,
+ buckets_data[i*bucket_size:(i+1)*bucket_size])
+ for i in range(num_buckets)]
+
+# Read the string table; the format doesn't explicitly communicate the
+# size of the string table (which is dumb), so assume it is the rest of
+# the file.
+f.seek(0, 2)
+strtable_size = f.tell() - strtable_offset
+f.seek(strtable_offset)
+
+if strtable_size == 0:
+raise SystemExit("error: %s: unable to read zero-sized string table"%(
+path,))
+strtable = f.read(strtable_size)
+
+if len(strtable) != strtable_size:
+raise SystemExit("error: %s: unable to read complete string table"%(
+path,))
+if strtable[-1] != '\0':
+raise SystemExit("error: %s: invalid string table in headermap" % (
+path,))
+
+return HeaderMap(num_entries, buckets, strtable)
+
+def __init__(self, num_entries, buckets, strtable):
+self.num_entries = num_entries
+self.buckets = buckets
+self.strtable = strtable
+
+def get_string(self, idx):
+if idx >= len(self.strtable):
+raise SystemExit("error: %s: invalid string index" % (
+path,))
+end_idx = self.strtable.index('\0', idx)
+return self.strtable[idx:end_idx]
+
+@property
+def mappings(self):
+for key_idx,prefix_idx,suffix_idx in self.buckets:
+if key_idx == 0:
+
[PATCH] D47157: Warning for framework headers using double quote includes
This revision was automatically updated to reflect the committed changes.
Closed by commit rL335184: Warning for framework headers using double quote
includes (authored by bruno, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47157?vs=149359&id=152177#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47157
Files:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/double-quotes/B.h
cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json
cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json
cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml
cfe/trunk/test/Modules/double-quotes.m
Index: cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json
+++ cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "A.h" : "A/A.h"
+}
+}
Index: cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
@@ -0,0 +1 @@
+// double-quotes/A.framework/Headers/A0.h
Index: cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
@@ -0,0 +1,6 @@
+#include "A0.h"
+#include "B.h"
+
+#include "X.h"
+
+int foo();
Index: cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
+++ cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+// double-quotes/A.framework/Modules/module.modulemap
+framework module A {
+ header "A.h"
+ header "A0.h"
+}
Index: cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml
+++ cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml
@@ -0,0 +1,28 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Modules",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+}
+ ]
+}
+ ]
+}
Index: cfe/trunk/test/Modules/Inputs/double-quotes/B.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/B.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/B.h
@@ -0,0 +1 @@
+// double-quotes/B.h
Index: cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
@@ -0,0 +1 @@
+#import "B.h" // Included from Z.h & A.h
Index: cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
+++ cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
@@ -0,0 +1,4 @@
+// double-quotes/flat-header-path/Z.modulemap
+framework module Z {
+ header "Z.h"
+}
Index: cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json
+++ cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json
@@ -0,0 +1,7 @@
+
+{
+ "mappings" :
+{
+ "X.h" : "X/X.h"
+}
+}
I
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno updated this revision to Diff 152198.
bruno added a comment.
Address Volodymyr's comments
https://reviews.llvm.org/D47301
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/HeaderSearch.cpp
test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
test/Modules/Inputs/framework-public-includes-private/a.hmap.json
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
test/Modules/Inputs/framework-public-includes-private/z.hmap.json
test/Modules/Inputs/framework-public-includes-private/z.yaml
test/Modules/framework-public-includes-private.m
Index: test/Modules/framework-public-includes-private.m
===
--- /dev/null
+++ test/Modules/framework-public-includes-private.m
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \
+// RUN: %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN: -F%S/Inputs/framework-public-includes-private \
+// RUN: -fsyntax-only %s \
+// RUN: 2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: public framework header includes private framework header 'A/APriv.h'
+// CHECK: public framework header includes private framework header 'A/APriv2.h'
+// CHECK: public framework header includes private framework header 'Z/ZPriv.h'
+
+#import "A.h"
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv.h'}}
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}}
+// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}}
Index: test/Modules/Inputs/framework-public-includes-private/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.yaml
@@ -0,0 +1,45 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'use-external-names' : 'false',
+ 'roots': [
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Headers",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "Z.h",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/PrivateHeaders",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "ZPriv.h",
+ 'external-contents': "TEST_DIR/flat-header-path/ZPriv.h"
+}
+ ]
+},
+{
+ 'type': 'directory',
+ 'name': "TEST_DIR/Z.framework/Modules",
+ 'contents': [
+{
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+},
+{
+ 'type': 'file',
+ 'name': "module.private.modulemap",
+ 'external-contents': "TEST_DIR/flat-header-path/Z.private.modulemap"
+}
+ ]
+}
+ ]
+}
Index: test/Modules/Inputs/framework-public-includes-private/z.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.hmap.json
@@ -0,0 +1,7 @@
+
+{
+ "mapping
[PATCH] D48367: [modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts
bruno added inline comments.
Comment at: clang/lib/Lex/HeaderSearch.cpp:285
// directory.
-loadSubdirectoryModuleMaps(SearchDirs[Idx]);
+if (ModMap.getLangOpts().ObjC1 || ModMap.getLangOpts().ObjC2)
+ loadSubdirectoryModuleMaps(SearchDirs[Idx]);
yamaguchi wrote:
> bruno wrote:
> > aprantl wrote:
> > > Are these flags also enabled in Objective-C++ mode?
> > Looks like all this logic was introduced in r177621 to allow the names of
> > modules to differ from the name of their subdirectory in the include path.
> >
> > Instead of having this to be based on the language, it's probably better if
> > we have it based on @import name lookup, which is the scenario where we
> > actually currently look more aggressively, did you try that path?
> >
> > This is also lacking a testcase, can you create one?
> > Are these flags also enabled in Objective-C++ mode?
> I think so from FrontendOptions.h:190
> `bool isObjectiveC() const { return Lang == ObjC || Lang == ObjCXX; }`
>
> > it's probably better if we have it based on @import name lookup
> I don't think I understood what you're saying, could you explain a bit more?
>
> > This is also lacking a testcase, can you create one?
> Sure!
> I don't think I understood what you're saying, could you explain a bit more?
This extra call to `loadSubdirectoryModuleMaps` was introduced in r177621 to
allow `@import SomeName` to work even if `SomeName` doesn't match a
subdirectory name. See the original commit for more details.
This means that it was never supposed to be called when the module is built via
`#include` or `#import`, which is what you are getting. That said, I believe
it's better if the condition for calling `loadSubdirectoryModuleMaps` checks
somehow if the lookup is coming from of an `@import` instead of checking the
language (we too don't want it to be called for ObjC when `#include` or
`#import` are used).
Comment at: clang/test/Modules/autoload-subdirectory.cpp:1
+// RUN: %clang -fmodules -fmodule-name=Foo -I %S/Inputs/autoload-subdirectory/
%s
+
Instead of `%clang`, please use `%clang_cc1` and `-verify` here.
https://reviews.llvm.org/D48367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno added a comment. Ping! https://reviews.llvm.org/D47301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output
bruno added a comment.
Hi Erik, thanks for improving this. Comments below.
Comment at: clang/include/clang/Lex/ModuleMap.h:650
void addHeader(Module *Mod, Module::Header Header,
- ModuleHeaderRole Role, bool Imported = false);
+ ModuleHeaderRole Role, StringRef OrigHeaderName = "",
+ bool Imported = false);
While here, can you add a `\param` entry for `Imported`?
Comment at: clang/lib/Frontend/DependencyFile.cpp:94
+ void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem) override {
+if (!llvm::sys::path::is_absolute(HeaderPath))
+ return;
Can you add a testecase for when `HeaderPath` isn't absolute?
Comment at: clang/lib/Lex/ModuleMap.cpp:1196
+// to the actual file. The callback should be notified of both.
+
+if (OrigHeaderName.empty())
Remove this empty line
Repository:
rC Clang
https://reviews.llvm.org/D53522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53228: [VFS] Remove 'ignore-non-existent-contents' attribute for YAML-based VFS.
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D53228 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM with some minor changes. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1008 + // Flag telling if we should iterate through ExternalFS or stop at the last + // RedirectingDirectoryEntry::iterator. + bool IterateExternalFS; Please use `///` instead of `//` to get doxygen's auto brief. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1010 + bool IterateExternalFS; + // Flag telling if we have switched to iterating through ExternalFS. + bool IsExternalFSCurrent = false; Same here Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1018 + // ...dispatching between RedirectingDirectoryEntry iteration and ExternalFS + // iteration. + std::error_code incrementImpl(bool IsFirstTime); Same here Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1020 + std::error_code incrementImpl(bool IsFirstTime); + // ...RedirectingDirectoryEntry iteration. + std::error_code incrementContent(bool IsFirstTime); Same here Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1022 + std::error_code incrementContent(bool IsFirstTime); + // ...ExternalFS iteration. + std::error_code incrementExternal(); Same here https://reviews.llvm.org/D50539 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55676: [Modules] Fix decl order for DeclsInPrototype
bruno added a comment. Ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55676/new/ https://reviews.llvm.org/D55676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55676: [Modules] Fix decl order for DeclsInPrototype
bruno added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55676/new/ https://reviews.llvm.org/D55676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57411: [ModuleDependencyCollector] Use llvm::sys::fs::real_path
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for working on this! LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57411/new/ https://reviews.llvm.org/D57411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.
bruno added a comment. Hi Volodymyr. Does this behavior changed after your VFS fallback change? https://reviews.llvm.org/D54245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54503: [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D54503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jkorous. LGTM https://reviews.llvm.org/D54245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD
bruno created this revision.
bruno added reviewers: rsmith, v.g.vassilev.
Herald added subscribers: dexonsmith, mgrang, jkorous.
A module signature hashes out all relevant content in a module in order
to guarantee that in a dep chain, an inconsistent module never gets
imported. When using implicit modules, clang is capable of rebuilding a
module in case its signature doesn't match what's expected in the
importer. However, when using PCHs + implicit modules, clang cannot
rebuild a imported module with a signature mismatch, since it doesn't
know how to rebuild PCHs.
Non-determinism in the module content, can lead to changes to the
signature of a PCM across compiler invocations that are expected to
produce the same result, causing:
- Build time penalties: rebuilding modules can be expensive.
- Failures: when using PCHs, leads to hard errors one cannot recover
from.
This patch address non-determinism when serializing DECL_CONTEXT_LEXICAL
and DECL_RECORD by sorting the decls by ID (which is supposed to be
deterministic across multiple invocations) prior to writing out the
records. Another approach would be to fix all the places that trigger
adding a Decl to a DeclContext in non deterministic order, but I
couldn't spot any after an audit, but probably didn't look at everything
since the number of different sites and nested calls that do that is
pretty high. Also, by fixing it right before serialization, we enforce
that future changes also don't mess up with the order.
This isn't new, I consistently tracked this behavior in clang all the
way back to 2016, where my testcase stops working for other reasons.
I have no sensible and reduced testcases to add to this patch.
rdar://problem/43442957
https://reviews.llvm.org/D54923
Files:
lib/Serialization/ASTCommon.h
lib/Serialization/ASTWriter.cpp
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3182,7 +3182,16 @@
uint64_t Offset = Stream.GetCurrentBitNo();
SmallVector KindDeclPairs;
- for (const auto *D : DC->decls()) {
+
+ // Decls have deterministic IDs, but they might show up in different order in
+ // a DeclContext. Make sure serialization gets the same order everytime by
+ // sorting the decls by ID.
+ SmallVector SortedDecls(DC->decls());
+ llvm::sort(SortedDecls, [](const Decl *L, const Decl *R) {
+return L->getID() < R->getID();
+ });
+
+ for (const auto *D : SortedDecls) {
KindDeclPairs.push_back(D->getKind());
KindDeclPairs.push_back(GetDeclRef(D));
}
Index: lib/Serialization/ASTCommon.h
===
--- lib/Serialization/ASTCommon.h
+++ lib/Serialization/ASTCommon.h
@@ -96,6 +96,7 @@
template void numberAnonymousDeclsWithin(const DeclContext *DC,
Fn Visit) {
unsigned Index = 0;
+ SmallVector DeclsToVisit;
for (Decl *LexicalD : DC->decls()) {
// For a friend decl, we care about the declaration within it, if any.
if (auto *FD = dyn_cast(LexicalD))
@@ -105,8 +106,17 @@
if (!ND || !needsAnonymousDeclarationNumber(ND))
continue;
-Visit(ND, Index++);
+DeclsToVisit.push_back(ND);
}
+
+ // Decls have deterministic IDs, but they might show up in different order in
+ // a DeclContext. Make sure serialization gets the same order everytime by
+ // sorting the decls by ID.
+ llvm::sort(DeclsToVisit, [](const NamedDecl *L, const NamedDecl *R) {
+return L->getID() < R->getID();
+ });
+ for (auto *ND : DeclsToVisit)
+Visit(ND, Index++);
}
} // namespace serialization
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3182,7 +3182,16 @@
uint64_t Offset = Stream.GetCurrentBitNo();
SmallVector KindDeclPairs;
- for (const auto *D : DC->decls()) {
+
+ // Decls have deterministic IDs, but they might show up in different order in
+ // a DeclContext. Make sure serialization gets the same order everytime by
+ // sorting the decls by ID.
+ SmallVector SortedDecls(DC->decls());
+ llvm::sort(SortedDecls, [](const Decl *L, const Decl *R) {
+return L->getID() < R->getID();
+ });
+
+ for (const auto *D : SortedDecls) {
KindDeclPairs.push_back(D->getKind());
KindDeclPairs.push_back(GetDeclRef(D));
}
Index: lib/Serialization/ASTCommon.h
===
--- lib/Serialization/ASTCommon.h
+++ lib/Serialization/ASTCommon.h
@@ -96,6 +96,7 @@
template void numberAnonymousDeclsWithin(const DeclContext *DC,
Fn Visit) {
unsigned Index = 0;
+ SmallVector DeclsToVisit;
for (Decl *LexicalD : DC->decls()) {
// For a friend decl, we care about the declaration within it, if an
[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD
bruno added a comment. Although we can't synthesize a testcase for inclusion in the patch. Here is how one should attempt to reproduce the problem in macOS 10.14. echo '@import Foundation;' > test.m clang -fmodules test.m -o test.o -c \ -fmodules-cache-path=/tmp/cache \ -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk mv /tmp/cache /tmp/cache1 clang -fmodules test.m -o test.o -c \ -fmodules-cache-path=/tmp/cache \ -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk mv /tmp/cache /tmp/cache2 HASH=`ls /tmp/cache1` for i in `find /tmp/cache1 -type f -iname "*.pcm"`; do F=`basename $i`; diff /tmp/cache1/$HASH/$F /tmp/cache2/$HASH/$F; done CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54923/new/ https://reviews.llvm.org/D54923 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55037: [-gmodules] Honor -fdebug-prefix-map in the debug info inside PCMs.
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55037/new/ https://reviews.llvm.org/D55037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions
bruno created this revision.
bruno added reviewers: rsmith, ahatanak, erik.pilkington.
Herald added subscribers: dexonsmith, jkorous.
Implement support for try-catch blocks in constexpr functions, as
proposed in http://wg21.link/P1002 and voted in San Diego for c++20.
The idea is that we can still never throw inside constexpr, so the catch
block is never entered. A try-catch block like this:
try { f(); } catch (...) { }
is then morally equivalent to just
{ f(); }
Same idea should apply for function/constructor try blocks.
rdar://problem/45530773
https://reviews.llvm.org/D55097
Files:
lib/AST/ExprConstant.cpp
lib/Sema/SemaDeclCXX.cpp
test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
test/CXX/drs/dr6xx.cpp
www/cxx_status.html
Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -953,13 +953,15 @@
Relaxations of constexpr restrictions
http://wg21.link/p1064r0";>P1064R0
- No
+ No
http://wg21.link/p1002r1";>P1002R1
+Clang 8
http://wg21.link/p1327r1";>P1327R1
+No
http://wg21.link/p1330r0";>P1330R0
Index: test/CXX/drs/dr6xx.cpp
===
--- test/CXX/drs/dr6xx.cpp
+++ test/CXX/drs/dr6xx.cpp
@@ -492,7 +492,10 @@
struct C {
constexpr C(NonLiteral);
constexpr C(NonLiteral, int) {} // expected-error {{not a literal type}}
-constexpr C() try {} catch (...) {} // expected-error {{function try block}}
+constexpr C() try {} catch (...) {}
+#if __cplusplus <= 201703L
+// expected-error@-2 {{function try block}}
+#endif
};
struct D {
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -verify -std=c++11 -fcxx-exceptions -Werror=c++1y-extensions %s
// RUN: %clang_cc1 -verify -std=c++1y -fcxx-exceptions -DCXX1Y %s
+// RUN: %clang_cc1 -verify -std=c++2a -fcxx-exceptions -DCXX1Y -DCXX2A %s
namespace N {
typedef char C;
@@ -49,7 +50,10 @@
// - its function-body shall not be a function-try-block;
struct U {
constexpr U()
-try // expected-error {{function try block not allowed in constexpr constructor}}
+try
+#ifndef CXX2A
+// expected-error@-2 {{function try block not allowed in constexpr constructor}}
+#endif
: u() {
} catch (...) {
throw;
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++1y-extensions %s
// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++1y -DCXX1Y %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++2a -DCXX1Y -DCXX2A %s
namespace N {
typedef char C;
@@ -78,7 +79,12 @@
};
struct T3 {
constexpr T3 &operator=(const T3&) const = default;
- // expected-error@-1 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#ifndef CXX2A
+ // expected-error@-2 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#else
+ // expected-warning@-4 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+ // expected-note@-5 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+#endif
};
#endif
struct U {
@@ -131,7 +137,10 @@
}
constexpr int DisallowedStmtsCXX1Y_3() {
// - a try-block,
- try {} catch (...) {} // expected-error {{statement not allowed in constexpr function}}
+ try {} catch (...) {}
+#ifndef CXX2A
+ // expected-error@-2 {{statement not allowed in constexpr function}}
+#endif
return 0;
}
constexpr int DisallowedStmtsCXX1Y_4() {
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -1900,6 +1900,27 @@
return false;
return true;
+ case Stmt::CXXTryStmtClass:
+if (!SemaRef.getLangOpts().CPlusPlus2a)
+ break;
+if (!Cxx1yLoc.isValid())
+ Cxx1yLoc = S->getBeginLoc();
+for (Stmt *SubStmt : S->children())
+ if (SubStmt &&
+ !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+ Cxx1yLoc))
+return false;
+return true;
+
+ case Stmt::CXXCatchStmtClass:
+if (!SemaRef.getLangOpts().CPlusPlus2a)
+ break;
+if (!Cxx1yLoc.isValid())
+ Cxx1yLoc = S->getB
[PATCH] D52967: Extend shelf-life by 70 years
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Out of curiosity, why? If it makes you happy though, go for it! LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52967/new/ https://reviews.llvm.org/D52967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
