[PATCH] D55159: Lex: Fix bug in __BASE_FILE__ implementation.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: rsmith.
Herald added a subscriber: llvm-commits.

__BASE_FILE__ previously expanded to "" instead of the name
of the main source file in files included using -include.

Also added test coverage for __BASE_FILE__ since there doesn't seem
to be any.


Repository:
  rL LLVM

https://reviews.llvm.org/D55159

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/Inputs/base_file.h
  clang/test/Preprocessor/base_file.c


Index: clang/test/Preprocessor/base_file.c
===
--- /dev/null
+++ clang/test/Preprocessor/base_file.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -include %S/Inputs/base_file.h -E %s | FileCheck %s
+
+// CHECK: {{[\\/]}}base_file.c"
+
+// CHECK: {{[\\/]}}base_file.c"
+#include "Inputs/base_file.h"
+
+// CHECK: {{[\\/]}}base_file.c"
+__BASE_FILE__
Index: clang/test/Preprocessor/Inputs/base_file.h
===
--- /dev/null
+++ clang/test/Preprocessor/Inputs/base_file.h
@@ -0,0 +1 @@
+__BASE_FILE__
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1505,24 +1505,11 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
 
-// __BASE_FILE__ is a GNU extension that returns the top of the presumed
-// #include stack instead of the current file.
-if (II == Ident__BASE_FILE__ && PLoc.isValid()) {
-  SourceLocation NextLoc = PLoc.getIncludeLoc();
-  while (NextLoc.isValid()) {
-PLoc = SourceMgr.getPresumedLoc(NextLoc);
-if (PLoc.isInvalid())
-  break;
-
-NextLoc = PLoc.getIncludeLoc();
-  }
-}
-
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
@@ -1531,6 +1518,12 @@
   OS << '"' << FN << '"';
 }
 Tok.setKind(tok::string_literal);
+  } else if (II == Ident__BASE_FILE__) {
+SmallString<128> FN =
+SourceMgr.getFileEntryForID(SourceMgr.getMainFileID())->getName();
+Lexer::Stringify(FN);
+OS << '"' << FN << '"';
+Tok.setKind(tok::string_literal);
   } else if (II == Ident__DATE__) {
 Diag(Tok.getLocation(), diag::warn_pp_date_time);
 if (!DATELoc.isValid())


Index: clang/test/Preprocessor/base_file.c
===
--- /dev/null
+++ clang/test/Preprocessor/base_file.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -include %S/Inputs/base_file.h -E %s | FileCheck %s
+
+// CHECK: {{[\\/]}}base_file.c"
+
+// CHECK: {{[\\/]}}base_file.c"
+#include "Inputs/base_file.h"
+
+// CHECK: {{[\\/]}}base_file.c"
+__BASE_FILE__
Index: clang/test/Preprocessor/Inputs/base_file.h
===
--- /dev/null
+++ clang/test/Preprocessor/Inputs/base_file.h
@@ -0,0 +1 @@
+__BASE_FILE__
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1505,24 +1505,11 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
 
-// __BASE_FILE__ is a GNU extension that returns the top of the presumed
-// #include stack instead of the current file.
-if (II == Ident__BASE_FILE__ && PLoc.isValid()) {
-  SourceLocation NextLoc = PLoc.getIncludeLoc();
-  while (NextLoc.isValid()) {
-PLoc = SourceMgr.getPresumedLoc(NextLoc);
-if (PLoc.isInvalid())
-  break;
-
-NextLoc = PLoc.getIncludeLoc();
-  }
-}
-
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
@@ -1531,6 +1518,12 @@
   OS << '"' << FN << '"';
 }
 Tok.setKind(tok::string_literal);
+  } else if (II == Ident__BASE_FILE__) {
+SmallString<128> FN =
+SourceMgr.getFileEntryForID(SourceMgr.getMainFileID())->getName();
+Lexer::Stringify(FN);
+OS << '"' << FN << '"';
+Tok.setKind(tok::string_literal);
   } else if (II == Ident__DATE__) {
 Diag

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 2 inline comments as done.
EricWF added a comment.

@jyknight wrote:

> And then use that to determine whether to add float128 to the union?


Note that `max_align_t` isn't a union of these types, but a struct containing 
all of them. This seems like a bug to me, but it's what GNU happens to do. God 
knows why?




Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

jyknight wrote:
> uweigand wrote:
> > jyknight wrote:
> > > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
> > > InitPreprocessor alongside the rest of the SIZEOF macros?
> > > 
> > > And then use that to determine whether to add float128 to the union? This 
> > > change, as is, will break on any target which is i386 but doesn't define 
> > > __float128, e.g. i386-unknown-unknown.
> > > 
> > > The only additional target which will be modified with that (that is: the 
> > > only other target which has a float128 type, but for which max_align 
> > > isn't already 16) is systemz-*-linux.
> > > 
> > > But I think that's actually indicating a pre-existing bug in the SystemZ 
> > > config -- it's configured for a 16-byte long double, with 8-byte 
> > > alignment, but the ABI doc seems to call for 16-byte alignment. +Ulrich 
> > > for comment on that.
> > That's a bug in the ABI doc which we'll fix once we get around to releasing 
> > an updated version.
> > 
> > long double on SystemZ must be 8-byte aligned, which is the maximum 
> > alignment of all standard types on Z, and this was chosen because 
> > historically the ABI only guarantees an 8-byte stack alignment, so 16-byte 
> > aligned standard types would be awkward.
> Then perhaps it's a bug that `__alignof__(__float128)` returns 16 with 
> `-target systemz-linux`?
@jyknight Ack. I'll add `__SIZEOF_FLOAT128__`.



Comment at: lib/Headers/__stddef_max_align_t.h:44
+#endif
 } max_align_t;
 #endif

rsmith wrote:
> I don't want to hold up the immediate fix in this patch for this, but... we 
> should move the definition of this type from the header into clang itself, 
> like we do for (say) `__builtin_va_list`, and here just define
> 
> `typedef __builtin_max_align_t max_align_t;`
> 
> That way Clang can synthesize a struct of whatever size and alignment 
> appropriate from an ABI perspective (or can use the relevant builtin type for 
> platforms that typedef `max_align_t` to a builtin type). That'd also remove 
> the need for an awkward factored-out header file here.
I'll try to implement this over the weekend. As long as I can land the Clang 
fix required for libc++ cleanup before the next release.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55057/new/

https://reviews.llvm.org/D55057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r348066 - [clangd] Recommit the "AnyScope" changes in requests.json by rCTE347753 (reverted by rCTE347792)

[Fangrui Song via cfe-commits]

Author: maskray
Date: Fri Nov 30 17:57:15 2018
New Revision: 348066

URL: http://llvm.org/viewvc/llvm-project?rev=348066&view=rev
Log:
[clangd] Recommit the "AnyScope" changes in requests.json by rCTE347753 
(reverted by rCTE347792)

This fixes IndexBenchmark tests.

Modified:
clang-tools-extra/trunk/test/clangd/Inputs/requests.json

Modified: clang-tools-extra/trunk/test/clangd/Inputs/requests.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/Inputs/requests.json?rev=348066&r1=348065&r2=348066&view=diff
==
--- clang-tools-extra/trunk/test/clangd/Inputs/requests.json (original)
+++ clang-tools-extra/trunk/test/clangd/Inputs/requests.json Fri Nov 30 
17:57:15 2018
@@ -1,7 +1,7 @@
-[{"Limit":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
-{"Limit":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""]},
-{"Limit":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""]},
-{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""]},
-{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""]},
-{"Limit":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::",
 "clang::", "clang::clangd::dex::"]},
-{"Limit":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}]
+[{"Limit":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"],
 "AnyScope":false},
+{"Limit":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""], "AnyScope":false},
+{"Limit":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""], "AnyScope":false},
+{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""], "AnyScope":false},
+{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::",
 ""], "AnyScope":false},
+{"Limit":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::",
 "clang::", "clang::clangd::dex::"],"AnyScope":false},
+{"Limit":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""],
 "AnyScope":false}]


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348065 - [Basic] Move DiagnosticsEngine::dump from .h to .cpp

[Fangrui Song via cfe-commits]

Author: maskray
Date: Fri Nov 30 17:43:05 2018
New Revision: 348065

URL: http://llvm.org/viewvc/llvm-project?rev=348065&view=rev
Log:
[Basic] Move DiagnosticsEngine::dump from .h to .cpp

The two LLVM_DUMP_METHOD methods have a undefined reference on 
clang::DiagnosticsEngine::DiagStateMap::dump.

tools/clang/tools/extra/clangd/benchmarks/IndexBenchmark links in
clangDaemon but does not link in clangBasic explicitly, which causes a
linker error "undefined symbol" in !NDEBUG + -DBUILD_SHARED_LIBS=on builds.

Move LLVM_DUMP_METHOD methods to .cpp to fix IndexBenchmark. They should
be unconditionally defined as they are also used by non-dump-method #pragma 
clang __debug diag_mapping

Modified:
cfe/trunk/include/clang/Basic/Diagnostic.h
cfe/trunk/lib/Basic/Diagnostic.cpp

Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=348065&r1=348064&r2=348065&view=diff
==
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Fri Nov 30 17:43:05 2018
@@ -486,10 +486,8 @@ public:
   DiagnosticsEngine &operator=(const DiagnosticsEngine &) = delete;
   ~DiagnosticsEngine();
 
-  LLVM_DUMP_METHOD void dump() const { DiagStatesByLoc.dump(*SourceMgr); }
-  LLVM_DUMP_METHOD void dump(StringRef DiagName) const {
-DiagStatesByLoc.dump(*SourceMgr, DiagName);
-  }
+  LLVM_DUMP_METHOD void dump() const;
+  LLVM_DUMP_METHOD void dump(StringRef DiagName) const;
 
   const IntrusiveRefCntPtr &getDiagnosticIDs() const {
 return Diags;

Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=348065&r1=348064&r2=348065&view=diff
==
--- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
+++ cfe/trunk/lib/Basic/Diagnostic.cpp Fri Nov 30 17:43:05 2018
@@ -89,6 +89,14 @@ DiagnosticsEngine::~DiagnosticsEngine()
   setClient(nullptr);
 }
 
+void DiagnosticsEngine::dump() const {
+  DiagStatesByLoc.dump(*SourceMgr);
+}
+
+void DiagnosticsEngine::dump(StringRef DiagName) const {
+  DiagStatesByLoc.dump(*SourceMgr, DiagName);
+}
+
 void DiagnosticsEngine::setClient(DiagnosticConsumer *client,
   bool ShouldOwnClient) {
   Owner.reset(ShouldOwnClient ? client : nullptr);


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55007: [Analyzer] Constraint Manager - Calculate Effective Range for Differences

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Is it an option to canonicalize the expression so that `B - A` was never stored 
in the first place? I.e., do this range intersection at the moment of writing 
the range, not at the moment of reading the range.

This could be implemented by, say, comparing symbol IDs for `A` and `B` and 
making sure that in every stored `SymSymExpr` the first symbol's ID is greater 
than the second symbol's ID.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55007/new/

https://reviews.llvm.org/D55007



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53329: Generate DIFile with main program if source is not available

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

F7630900: clang-crash-x86-2.tar 

@scott.linder Actually clang can get source for remapped files. I just uploaded 
another test tarball which may help you debug the issue.
The test script is changed as well since recent llvm/clang requiring linking 
with -ltinfo.

Just give you a summary here.

  -bash-4.2$ cat ttest.h
  
  #define BPF_PERF_OUTPUT2(_name) \
  struct _name##_table_t { \
int key; \
unsigned leaf; \
int (*perf_submit) (void *, void *, unsigned); \
int (*perf_submit_skb) (void *, unsigned, void *, unsigned); \
unsigned max_entries; \
  }; \
  __attribute__((section("maps/perf_output"))) \
  struct _name##_table_t _name = { .max_entries = 0 }
  
  int g;
  -bash-4.2$ cat ttest1.c
  #include "ttest.h"
  
  int main() { return g; }
  -bash-4.2$ cat ttest2.c
  #include "ttest.h"
  
  BPF_PERF_OUTPUT2(probe);
  
  int main() { return g; }
  -bash-4.2$

The header file ttest.h is remapped.
For remapped file ttest1.c, the clang can get the source properly.
For remapped file ttest2.c, the clang cannot get the source.
The difference between two files are macro usage.

  -bash-4.2$ diff ttest1.c ttest2.c
  2a3,4
  > BPF_PERF_OUTPUT2(probe);
  > 
  -bash-4.2$

So macro existence may somehow prevent availability of remapped file source.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53329/new/

https://reviews.llvm.org/D53329



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348062 - Relax test to also work on Windows.

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Fri Nov 30 17:30:00 2018
New Revision: 348062

URL: http://llvm.org/viewvc/llvm-project?rev=348062&view=rev
Log:
Relax test to also work on Windows.

Modified:
cfe/trunk/test/CodeGenCXX/debug-prefix-map-lambda.cpp

Modified: cfe/trunk/test/CodeGenCXX/debug-prefix-map-lambda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-prefix-map-lambda.cpp?rev=348062&r1=348061&r2=348062&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-prefix-map-lambda.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-prefix-map-lambda.cpp Fri Nov 30 17:30:00 
2018
@@ -4,7 +4,7 @@
 template  void b(T) {}
 void c() {
   // CHECK: !DISubprogram(name: "b<(lambda at
-  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  SOURCE_ROOT
   // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
   b([]{});
 }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Looks fine to me; please don't let //me// block this any further. :) Someone 
else, e.g. @aaron.ballman, should be the one to accept it, though.




Comment at: include/clang/AST/NestedNameSpecifier.h:220
+  void print(raw_ostream &OS, const PrintingPolicy &Policy,
+ bool ResolveTemplateArguments = false) const;
 

Peanut gallery says: Should `ResolveTemplateArguments` really be here, or 
should it be a property of the `PrintingPolicy` the same way e.g. 
`ConstantsAsWritten` is a property of the `PrintingPolicy`?  I don't know what 
a `PrintingPolicy` is, really, but I know that I hate defaulted boolean 
function parameters with a passion. ;)

Furthermore, I note that the doc-comment for `ConstantsAsWritten`, at line ~207 
of include/clang/AST/PrettyPrinter.h, is nonsensical and maybe someone should 
give it some love. (That is totally not //your// problem, though.)



Comment at: lib/Sema/SemaTemplate.cpp:3071
+  printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
+}
+return;

Checking my understanding: Am I correct that this code currently does not 
pretty-print

static_assert(std::is_same(), "");

(creating an object of the trait type and then using its constexpr `operator 
bool` to convert it to bool)? This is a rare idiom and doesn't need to be 
supported AFAIC.



Comment at: test/SemaCXX/static-assert.cpp:143
+void foo2() {
+  static_assert(::ns::NestedTemplates1::NestedTemplates2::template 
NestedTemplates3::value, "message");
+  // expected-error@-1{{static_assert failed due to requirement 
'::ns::NestedTemplates1::NestedTemplates2::NestedTemplates3::value' "message"}}

Looking at this example, I thought of one more case to test.
```
static_assert(std::is_same_v, "");
```
That is, I'd like to see a test that verifies whether `T[sizeof(T)]` 
pretty-prints as `float[sizeof(float)]`, or `float[4]`, or something else. (I 
don't really care what it prints as, as long as it's not completely crazy, and 
as long as it doesn't regress in future releases.)



Comment at: test/SemaCXX/static-assert.cpp:164
+  static_assert(std::is_const::value, "message");
+  // expected-error@-1{{type 'int' cannot be used prior to '::' because it has 
no members}}
+}

I guess I meant more like
```
struct S {};
void bar() {
foo(S{});
}
```
```
error: no member named 'iterator' in 'S'
static_assert(std::is_same_v);
  ~~~^
```
but as neither version goes through your codepath anyway, what you've got is OK.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54903/new/

https://reviews.llvm.org/D54903



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Thanks, nice catch!

It seems that the `ReportDoubleDelete()` thing was never used for reporting 
double-delete, but it was used for some strange check when a destructor is 
called. Is that old code even correct?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54834/new/

https://reviews.llvm.org/D54834



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55151/new/

https://reviews.llvm.org/D55151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

[Vedant Kumar via Phabricator via cfe-commits]

vsk marked an inline comment as done.
vsk added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1037
   if (hasExportSymbolDirective(Args)) {
-addExportedSymbol(CmdArgs, "___llvm_profile_filename");
-addExportedSymbol(CmdArgs, "___llvm_profile_raw_version");
-addExportedSymbol(CmdArgs, "_lprofCurFilename");
+if (needsGCovInstrumentation(Args)) {
+  addExportedSymbol(CmdArgs, "___gcov_flush");

ributzka wrote:
> Are the symbols mutually exclusive?
Yes, since the runtime is a .a, the gcov-specific symbols aren't pulled in when 
clang-cov is enabled and vice versa.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55151/new/

https://reviews.llvm.org/D55151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1037
   if (hasExportSymbolDirective(Args)) {
-addExportedSymbol(CmdArgs, "___llvm_profile_filename");
-addExportedSymbol(CmdArgs, "___llvm_profile_raw_version");
-addExportedSymbol(CmdArgs, "_lprofCurFilename");
+if (needsGCovInstrumentation(Args)) {
+  addExportedSymbol(CmdArgs, "___gcov_flush");

Are the symbols mutually exclusive?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55151/new/

https://reviews.llvm.org/D55151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Yay. Wow. Cool. Thanks.

I heard that previously there was an explicit desire to avoid doxygen comments 
in checkers because they're quite useless because the checker is usually all in 
one file and doesn't have any clients that are interested in calling its 
methods directly.

But now that our checkers are split into files and start interacting with each 
other, this argument no longer holds.

So, well, thanks a lot.




Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:16
+//   * MallocChecker
+//   Despite it's name, it models all sorts of memory allocations and
+//   de- or reallocation, including but not limited to malloc, free,

~~it's~~ its



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:23
+//   MallocChecker, but do not enable MallocChecker's reports (more details
+//   to follow around it's field, ChecksEnabled).
+//   It also has a boolean "Optimistic" checker option, which if set to 
true

~~it's~~ its



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:283-300
+  mutable IdentifierInfo *II_alloca = nullptr, *II_win_alloca = nullptr,
+ *II_malloc = nullptr, *II_free = nullptr,
+ *II_realloc = nullptr, *II_calloc = nullptr,
+ *II_valloc = nullptr, *II_reallocf = nullptr,
+ *II_strndup = nullptr, *II_strdup = nullptr,
+ *II_win_strdup = nullptr, *II_kmalloc = nullptr,
+ *II_if_nameindex = nullptr,

Not now of course, but see also D45458. It'd be great to have call descriptions 
here, so that it looked like
```lang=c++
CallDescription alloca("alloca"),
win_alloca("win_alloca"),
malloc("malloc"),
// ...
```
..and no initialization necessary later.

(we can even wrap them into a macro so that we didn't have to write names 
twice).



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:422
+  ///
+  /// \param [in] CE The expression that allocates memory.
+  /// \param [in] IndexOfSizeArg Index of the argument that specifies the size

MTC wrote:
> `CE` typo?
Looks fine. It's the name of the parameter in the doxygen syntax.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:592
+  /// \param [in] CE The expression that reallocated memory
+  /// \param [in] State The \c ProgramState right before reallocation.
+  /// \returns The programstate right after allocation.

MTC wrote:
> `before realloction` typo?
Seems fine.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1087
   if (FD->getKind() == Decl::Function) {
-initIdentifierInfo(C.getASTContext());
+MemFunctionInfo.initIdentifierInfo(C.getASTContext());
 IdentifierInfo *FunI = FD->getIdentifier();

Szelethus wrote:
> MTC wrote:
> > Szelethus wrote:
> > > MTC wrote:
> > > > If I not wrong, `MemFunctionInfo` is mainly a class member of 
> > > > `MallocChecker` that hold a bunch of memory function identifiers and 
> > > > provide corresponding helper APIs. And we should pick a proper time to 
> > > > initialize it.
> > > > 
> > > > I want to known why you call `initIdentifierInfo()` in 
> > > > `checkPostStmt(const CallExpr *E,)`, this callback is called after 
> > > > `checkPreCall(const CallEvent &Call, )` in which we need 
> > > > `MemFunctionInfo`.
> > > Well, I'd like to know too! I think lazy initializing this struct creates 
> > > more problems that it solves, but I wanted to draw a line somewhere in 
> > > terms of how invasive I'd like to make this patch.
> > How about put the initialization stuff into constructor? Let the 
> > constructor do the initialization that it should do, let `register*()` do 
> > its registration, like setting `setOptimism` and so on.
> Interestingly, `MemFunctionInfo` is not always initialized, so a performance 
> argument can be made here. I specifically checked whether there's any point 
> in doing this hackery, and the answer is... well, some. I'll probably touch 
> on these in a followup patch.
Lazy initialization of identifiers is kinda perceived as a fairly worthless 
optimization. Hence `CallDescription`.

Also it cannot be done during registration because the AST is not constructed 
yet at this point. Well, probably it can be done anyway because the empty 
identifier table is already there in the `ASTContext` and we can always eagerly 
add a few items into it, but it still sounds scary.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l

r348060 - Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Fri Nov 30 16:24:27 2018
New Revision: 348060

URL: http://llvm.org/viewvc/llvm-project?rev=348060&view=rev
Log:
Honor -fdebug-prefix-map when creating function names for the debug info.

This adds a callback to PrintingPolicy to allow CGDebugInfo to remap
file paths according to -fdebug-prefix-map. Otherwise the debug info
(particularly function names for C++ lambdas) may contain paths that
should have been remapped in the debug info.



Differential Revision: https://reviews.llvm.org/D55137

Added:
cfe/trunk/test/CodeGenCXX/debug-prefix-map-lambda.cpp
Modified:
cfe/trunk/include/clang/AST/PrettyPrinter.h
cfe/trunk/lib/AST/TypePrinter.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h

Modified: cfe/trunk/include/clang/AST/PrettyPrinter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/PrettyPrinter.h?rev=348060&r1=348059&r2=348060&view=diff
==
--- cfe/trunk/include/clang/AST/PrettyPrinter.h (original)
+++ cfe/trunk/include/clang/AST/PrettyPrinter.h Fri Nov 30 16:24:27 2018
@@ -38,21 +38,20 @@ public:
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(false), ConstantsAsWritten(false),
+SuppressImplicitBase(false), FullyQualifiedName(false),
+RemapFilePaths(false) {}
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -225,6 +224,12 @@ struct PrintingPolicy {
   /// When true, print the fully qualified name of function declarations.
   /// This is the opposite of SuppressScope and thus overrules it.
   unsigned FullyQualifiedName : 1;
+
+  /// Whether to apply -fdebug-prefix-map to any file paths.
+  unsigned RemapFilePaths : 1;
+
+  /// When RemapFilePaths is true, this function performs the action.
+  std::function remapPath;
 };
 
 } // end namespace clang

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=348060&r1=348059&r2=348060&view=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Fri Nov 30 16:24:27 2018
@@ -1157,9 +1157,13 @@ void TypePrinter::printTag(TagDecl *D, r
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=348060&r1=348059&r2=348060&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Nov 30 16:24:27 2018
@@ -235,6 +235,9 @@ PrintingPolicy CGDebugInfo::getPrintingP
   if (CGM.

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348060: Honor -fdebug-prefix-map when creating function 
names for the debug info. (authored by adrian, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55137?vs=176194&id=176225#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137

Files:
  include/clang/AST/PrettyPrinter.h
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-prefix-map-lambda.cpp

Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -38,21 +38,20 @@
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(false), ConstantsAsWritten(false),
+SuppressImplicitBase(false), FullyQualifiedName(false),
+RemapFilePaths(false) {}
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -225,6 +224,12 @@
   /// When true, print the fully qualified name of function declarations.
   /// This is the opposite of SuppressScope and thus overrules it.
   unsigned FullyQualifiedName : 1;
+
+  /// Whether to apply -fdebug-prefix-map to any file paths.
+  unsigned RemapFilePaths : 1;
+
+  /// When RemapFilePaths is true, this function performs the action.
+  std::function remapPath;
 };
 
 } // end namespace clang
Index: test/CodeGenCXX/debug-prefix-map-lambda.cpp
===
--- test/CodeGenCXX/debug-prefix-map-lambda.cpp
+++ test/CodeGenCXX/debug-prefix-map-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   -fdebug-prefix-map=%S=/SOURCE_ROOT %s -emit-llvm -o - | FileCheck %s
+
+template  void b(T) {}
+void c() {
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
+  b([]{});
+}
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1157,9 +1157,13 @@
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -235,6 +235,9 @@
   if (CGM.getCodeGenOpts().EmitCodeView)
 PP.MSVCFormatting = true;
 
+  // Apply -fdebug-prefix-map.
+  PP.RemapFilePaths = true;
+  PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };
   return PP;
 }
 
Index: lib/CodeGen/CGDebugInfo.h
=

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl marked an inline comment as done.
aprantl added inline comments.



Comment at: test/CodeGenCXX/debug-prefix-map-lambda.cpp:7
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"

probinson wrote:
> I've never liked tests that hard-code the test file name. FileCheck lets you 
> define variables on the RUN line, is there a lit substitution that will 
> return just the basename?
Looks like there is not LIT substitution for the filename without the path. At 
least not a documented one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54466: [Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Huh, gotcha :) The nomenclature is mostly correct, just feels weird.

Super-region of a region is a larger region. Sub-region is a smaller region. 
Base region is the largest non-memspace superregion, so it's a //larger// 
region.

Derived class object is a large object. Base class object is a small object 
within the derived class object, so it should be represented by a //smaller// 
region.

Hence we have `CXXBaseObjectRegion` which is a small sub-region that 
corresponds to the object of the base class within the object of a derived 
class. Which means that yes, you obtain the base region by removing 
`CXXBaseObjectRegion` layers (i.e., making the region larger), and that is fine 
because "base region" in our terminology means a completely different thing.

Now, right, there's the `CXXDerivedObjectRegion` that i added recently. It 
represents the region of a derived object if its super-object isn't a 
`CXXBaseObjectRegion` (otherwise we can just unwrap the `CXXBaseObjectRegion`). 
So it kinda sounds like a sub-region that is larger than its superregion, which 
contradicts the tradition. But this is fine because if the program under 
analysis is valid (which we assume unless we can prove the opposite), the only 
valid use case for `CXXDerivedObjectRegion` is to place it on top of a 
`SymbolicRegion` which points to an object of unknown size. So we kinda don't 
really know what's larger here. Also, that's not the only example of the 
situation when a sub-region is not a sub-segment of its super-region. We can 
always add an `ElementRegion` with an arbitrary offset over almost any other 
region and it'll be equally wonky (i.e., a valid program shouldn't make us do 
that, but if it does, then, well, it's not surprising that weird things are 
happening).




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:121
 
+  const MemRegion *getMostDerivedObjectRegion() const;
+

Let's start writing doxygen thingies!



Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:1182-1188
+  while (true) {
+if (R->getKind() == MemRegion::CXXBaseObjectRegionKind) {
+R = cast(R)->getSuperRegion();
+continue;
+}
+break;
+  }

My attempt:
```lang=c++
while (const auto *BR = dyn_cast(R))
  R = BR->getSuperRegion();
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54466/new/

https://reviews.llvm.org/D54466



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

[Vedant Kumar via Phabricator via cfe-commits]

vsk created this revision.
vsk added reviewers: ributzka, steven_wu, calixte, marco-c.

Make sure that symbols needed to implement runtime support for gcov are
exported when using an export list on Darwin.

Without the clang driver exporting these symbols, the linker hides them,
resulting in tapi verification failures.

rdar://45944768


https://reviews.llvm.org/D55151

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld.c
  compiler-rt/test/profile/instrprof-darwin-exports.c

Index: compiler-rt/test/profile/instrprof-darwin-exports.c
===
--- compiler-rt/test/profile/instrprof-darwin-exports.c
+++ compiler-rt/test/profile/instrprof-darwin-exports.c
@@ -8,6 +8,9 @@
 // RUN: %clang_profgen -Werror -fcoverage-mapping -Wl,-exported_symbols_list,%t.exports -o %t %s 2>&1 | tee -a %t.log
 // RUN: cat %t.log | count 0
 
+// RUN: %clang -Werror -Wl,-exported_symbols_list,%t.exports --coverage -o %t.gcov %s | tee -a %t.gcov.log
+// RUN: cat %t.gcov.log | count 0
+
 // The default set of weak external symbols should match the set of symbols
 // exported by clang. See Darwin::addProfileRTLibs.
 
@@ -16,4 +19,9 @@
 // RUN: nm -jUg %t > %t.clang.exports
 // RUN: diff %t.default.exports %t.clang.exports
 
+// RUN: %clang -Werror --coverage -o %t.gcov.default %s
+// RUN: nm -jUg %t.gcov | grep -v __mh_execute_header > %t.gcov.exports
+// RUN: nm -jUg %t.gcov.default | grep -v __mh_execute_header > %t.gcov.default.exports
+// RUN: diff %t.gcov.default.exports %t.gcov.exports
+
 int main() {}
Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -341,10 +341,22 @@
 // RUN: FileCheck -check-prefix=PROFILE_EXPORT %s < %t.log
 // PROFILE_EXPORT: "-exported_symbol" "___llvm_profile_filename" "-exported_symbol" "___llvm_profile_raw_version" "-exported_symbol" "_lprofCurFilename"
 //
-// RUN: %clang -target x86_64-apple-darwin12 -fprofile-instr-generate -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -fprofile-instr-generate --coverage -### %t.o 2> %t.log
 // RUN: FileCheck -check-prefix=NO_PROFILE_EXPORT %s < %t.log
 // NO_PROFILE_EXPORT-NOT: "-exported_symbol"
 //
+// RUN: %clang -target x86_64-apple-darwin12 --coverage -exported_symbols_list /dev/null -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=GCOV_EXPORT %s < %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -fprofile-arcs -Wl,-exported_symbols_list,/dev/null -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=GCOV_EXPORT %s < %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -fprofile-arcs -Wl,-exported_symbol,foo -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=GCOV_EXPORT %s < %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -fprofile-arcs -Xlinker -exported_symbol -Xlinker foo -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=GCOV_EXPORT %s < %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -fprofile-arcs -Xlinker -exported_symbols_list -Xlinker /dev/null -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=GCOV_EXPORT %s < %t.log
+// GCOV_EXPORT: "-exported_symbol" "___gcov_flush"
+//
 // Check that we can pass the outliner down to the linker.
 // RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
 // RUN:   %clang -target arm64-apple-darwin -moutline -### %t.o 2> %t.log
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1034,11 +1034,17 @@
   // runtime, automatically export symbols necessary to implement some of the
   // runtime's functionality.
   if (hasExportSymbolDirective(Args)) {
-addExportedSymbol(CmdArgs, "___llvm_profile_filename");
-addExportedSymbol(CmdArgs, "___llvm_profile_raw_version");
-addExportedSymbol(CmdArgs, "_lprofCurFilename");
+if (needsGCovInstrumentation(Args)) {
+  addExportedSymbol(CmdArgs, "___gcov_flush");
+  addExportedSymbol(CmdArgs, "_flush_fn_list");
+  addExportedSymbol(CmdArgs, "_writeout_fn_list");
+} else {
+  addExportedSymbol(CmdArgs, "___llvm_profile_filename");
+  addExportedSymbol(CmdArgs, "___llvm_profile_raw_version");
+  addExportedSymbol(CmdArgs, "_lprofCurFilename");
+  addExportedSymbol(CmdArgs, "_lprofMergeValueProfData");
+}
 addExportedSymbol(CmdArgs, "_lprofDirMode");
-addExportedSymbol(CmdArgs, "_lprofMergeValueProfData");
   }
 }
 
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -403,19 +403,23 @@
 }
 
 bool ToolChain::needsProfileRT(const ArgList &Args) {
-  if (Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
-   fal

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis requested changes to this revision.
eugenis added a comment.
This revision now requires changes to proceed.

Sorry for the delay.
This is wrong, static linking is NOT supported.
You could be confusing it with static linking of asan runtime library to an 
executable - that is and has always been the default.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55066/new/

https://reviews.llvm.org/D55066



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In D55101#1315294 , @benhamilton wrote:

> > Would you be okay with landing this fix and if we get any further reports 
> > for Objective-C++ sources then we can suppress it in all C++/Objective-C++ 
> > sources? I think there is enough value to enforcing the naming conventions 
> > on non-namespaced C functions in Objective-C++ to justify a simple followup 
> > fix. If other issues are reported after this then I also agree that 
> > enforcement in Objective-C++ sources may incur more overhead than it's 
> > worth.
>
> I'm not against it, but we've already disabled the majority of Objective-C 
> checks for Objective-C++ code, so I don't think this one should apply either.


Within the module named "google-module", I only see that AvoidCStyleCastsCheck 
is disabled for Objective-C++. That check is probably disabled because C-style 
casts on Objective-C types in Objective-C++ code are not seen as a major issue 
and the cost of refactoring code to use or not use C-style casts as code is 
refactored between Objective-C and Objective-C++ sources might be considered 
high.
Within the module named "objc-module", it appears that all checks are currently 
enabled for Objective-C++ code.
Looking across all the clang-tidy modules, AvoidCStyleCastsCheck might actually 
be the only check that is specifically disabled for Objective-C++ (although the 
`grep` I used to posit this assumes that any check that is suppressed in 
Objective-C++ must contain the string `getLangOpts().ObjC`: `grep -r 
"getLangOpts().ObjC" clang-tidy/`).

There are certainly cases where the heterogeneity of Objective-C++ encourages 
disabling a check due to inability to distinguish between whether C++ or 
Objective-C practices should be contextually dominant. I think that 
google-objc-function-naming should be able to cleanly separate declarations 
that should follow C++ or Objective-C patterns. If further bugs are reported 
after this is submitted then I am more willing to concede that I was overly 
optimistic. If it's alright with you, I would prefer to land this fix to give 
google-objc-function-naming another attempt at proper enforcement in 
Objective-C++ and consider disabling it in all C++ variants as a followup to 
further bug reports in Objective-C++.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54437: [analyzer][NFC] Merge ClangCheckerRegistry to CheckerRegistry

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Yay!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54437/new/

https://reviews.llvm.org/D54437



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54436: [analyzer][NFC] Move CheckerRegistry from the Core directory to Frontend

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Thanks for the cleanup!

One thing to test here is whether this works (or, at least, builds) when 
compiling clang without the analyzer.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54436/new/

https://reviews.llvm.org/D54436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348053 - Revert "Revert r347417 "Re-Reinstate 347294 with a fix for the failures.""

[Fangrui Song via cfe-commits]

Author: maskray
Date: Fri Nov 30 15:41:18 2018
New Revision: 348053

URL: http://llvm.org/viewvc/llvm-project?rev=348053&view=rev
Log:
Revert "Revert r347417 "Re-Reinstate 347294 with a fix for the failures.""

It seems the two failing tests can be simply fixed after r348037

Fix 3 cases in Analysis/builtin-functions.cpp
Delete the bad CodeGen/builtin-constant-p.c for now

Added:
cfe/trunk/test/CodeGenCXX/builtin-constant-p.cpp
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
cfe/trunk/test/Sema/builtins.c
cfe/trunk/test/SemaCXX/compound-literal.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=348053&r1=348052&r2=348053&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Fri Nov 30 15:41:18 2018
@@ -583,7 +583,8 @@ public:
   /// this function returns true, it returns the folded constant in Result. If
   /// the expression is a glvalue, an lvalue-to-rvalue conversion will be
   /// applied.
-  bool EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx) const;
+  bool EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx,
+bool InConstantContext = false) const;
 
   /// EvaluateAsBooleanCondition - Return true if this is a constant
   /// which we can fold and convert to a boolean condition using
@@ -600,7 +601,7 @@ public:
 
   /// EvaluateAsInt - Return true if this is a constant which we can fold and
   /// convert to an integer, using any crazy technique that we want to.
-  bool EvaluateAsInt(llvm::APSInt &Result, const ASTContext &Ctx,
+  bool EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
  SideEffectsKind AllowSideEffects = SE_NoSideEffects) 
const;
 
   /// EvaluateAsFloat - Return true if this is a constant which we can fold and
@@ -901,10 +902,15 @@ public:
 
 /// ConstantExpr - An expression that occurs in a constant context.
 class ConstantExpr : public FullExpr {
-public:
   ConstantExpr(Expr *subexpr)
 : FullExpr(ConstantExprClass, subexpr) {}
 
+public:
+  static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+assert(!isa(E));
+return new (Context) ConstantExpr(E);
+  }
+
   /// Build an empty constant expression wrapper.
   explicit ConstantExpr(EmptyShell Empty)
 : FullExpr(ConstantExprClass, Empty) {}
@@ -3087,8 +3093,8 @@ inline Expr *Expr::IgnoreImpCasts() {
   while (true)
 if (ImplicitCastExpr *ice = dyn_cast(e))
   e = ice->getSubExpr();
-else if (ConstantExpr *ce = dyn_cast(e))
-  e = ce->getSubExpr();
+else if (FullExpr *fe = dyn_cast(e))
+  e = fe->getSubExpr();
 else
   break;
   return e;

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=348053&r1=348052&r2=348053&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Nov 30 15:41:18 2018
@@ -6388,7 +6388,7 @@ ExpectedStmt ASTNodeImporter::VisitConst
   Expr *ToSubExpr;
   std::tie(ToSubExpr) = *Imp;
 
-  return new (Importer.getToContext()) ConstantExpr(ToSubExpr);
+  return ConstantExpr::Create(Importer.getToContext(), ToSubExpr);
 }
 
 ExpectedStmt ASTNodeImporter::VisitParenExpr(ParenExpr *E) {

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=348053&r1=3480

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a subscriber: alexfh.
NoQ added a comment.

The code looks good, but i vaguely remember that we might accidentally break 
clang-tidy integration that uses this API to enable/disable specific checkers 
via `-checks=-analyzer-...`.

*summons @alexfh*


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54401/new/

https://reviews.llvm.org/D54401



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54149: [Analyzer] [WIP] Standard C++ library functions checker for the std::find() family

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added a subscriber: gamesh411.

Such checker is useful. It'd be great to add a lot of "pure" functions into it, 
simply for the sake of pointing out that they have no side effects, i.e. avoid 
unnecessary invalidation.

The bound-to-argument constraint is useful. It avoids the alpha-renaming 
problem by assigning the correct value right away instead of stuffing an 
aliasing condition into the solver.

I still believe that the idea to split the state on `std::find` is 
questionable, no matter how exactly it's implemented. `std::find` does more 
than tells whether the element is in the container, so calling it just for its 
other effect (to obtain the iterator to the element) when you already know that 
the element is in the container is not an indication of a dead code, even if it 
is a bad code smell in most cases. I think it is very likely that people will 
come to us with false positives of that kind and we'll have to revert this 
behavior. Would you be OK to keep support for `std::find()` under an option, 
off by default but available when the user opts into lint checks? This could be 
implemented by making a new check kind (eg., 
`CK_OptimisticStdCXXLibraryFunctionsChecker`).

Random thought: i still think that implementing the "object value id" thing (a 
symbol-like id that is associated with the memory region of the object and gets 
invalidated when the object is logically mutated but gets copied as-is to the 
new object upon copy/move-constructors or assignments) is a good way to 
refactor all these things. But this work seems orthogonal to what you did here 
(i.e., we'll have to perform the copy of the "id" here, but that's just a 
separate task). So this patch shouldn't be blocked on that.

Another random thought: it'd be great to make a bug reporter visitor that 
highlights state splits produced by this checker. Eg., in your case it would 
produce an event note that says something like "Assuming object is not present 
in iterator range". Otherwise the user would be unable to understand why do you 
think that the iterator is bad. Or for, say, `ispunct()` it would say "Assuming 
character is punctuation symbol".




Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:159-161
+ProgramStateRef
+applyAsBoundToArgument(ProgramStateRef State, const CallEvent &Call,
+   const FunctionSummaryTy &Summary) const;

I think it's important to document that bound-to-argument should always be used 
instead of compares-to-argument when the comparison operator is == and the 
constraint is applied to return values of fully evaluated functions.



Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:375-376
+  OtherV = SVB.evalCast(OtherV, T, OtherT);
+  State = State->BindExpr(Call.getOriginExpr(), Call.getLocationContext(),
+  OtherV);
+  return State;

H.

`std::find` is a function that returns a C++ object by value.

For such functions it is not enough to bind the returned prvalue of the object. 
It is important to realize that while it is an equal iterator, it's not the 
same object. Which is why it is necessary to foresee the storage for the newly 
constructed iterator, so that destruction/materialization/copy elision worked 
correctly. See the respective logic in `ExprEngine::bindReturnValue()`:

```
  lang=c++
   611   if (auto RTC = getCurrentCFGElement().getAs()) {
   612 // Conjure a temporary if the function returns an object by value.
   613 SVal Target;
   614 assert(RTC->getStmt() == Call.getOriginExpr());
   615 EvalCallOptions CallOpts; // FIXME: We won't really need those.
   616 std::tie(State, Target) =
   617 prepareForObjectConstruction(Call.getOriginExpr(), State, LCtx,
   618  RTC->getConstructionContext(), 
CallOpts);
   // ...
   627   } // ...
```

It's bad that checkers currently need to do that manually when they implement 
`evalCall()` for C++ functions that return objects. We need to come up with a 
better API, eg. `State->BindReturnValue(same arguments)` that prepares for 
object construction (i.e., move the part of the functionality of 
`ExprEngine::bindReturnValue()` that's responsible for binding into a 
`ProgramState` method, and only keep the functionality that's responsible for 
conjuring the value).



Comment at: test/Analysis/std-cxx-library-functions.cpp:7-11
+void test_find(std::vector V, int n) {
+  const std::vector::iterator b = V.begin(), e = V.end();
+  clang_analyzer_eval(std::find(b, e, n) == e); // expected-warning{{TRUE}}
+  // expected-warning@-1{{FALSE}}
+}

I think that's a great standalone test.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54149/new/

https://reviews.llvm.org/D54149



___
cfe-commit

[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 176214.
ahatanak added a comment.
Herald added a subscriber: jkorous.

Rebase & ping.

I've reverted the changes I made to test/SemaCUDA/call-host-fn-from-device.cu 
since r342749 fixed the overload resolution bug.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47757/new/

https://reviews.llvm.org/D47757

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CXX/drs/dr2xx.cpp
  test/SemaCXX/unavailable_aligned_allocation.cpp

Index: test/SemaCXX/unavailable_aligned_allocation.cpp
===
--- test/SemaCXX/unavailable_aligned_allocation.cpp
+++ test/SemaCXX/unavailable_aligned_allocation.cpp
@@ -124,7 +124,73 @@
 // expected-note@-20 2 {{if you supply your own aligned allocation functions}}
 #endif
 
-// No errors if user-defined aligned allocation functions are available.
+// Test that diagnostics are produced when an unavailable aligned deallocation
+// function is called from a deleting destructor.
+struct alignas(256) OveralignedS2 {
+  int a[4];
+  virtual ~OveralignedS2();
+};
+
+OveralignedS2::~OveralignedS2() {}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+#if defined(IOS)
+// expected-error@-6 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on iOS 11 or newer}}}
+// expected-note@-7 {{if you supply your own aligned allocation functions}}
+#elif defined(TVOS)
+// expected-error@-9 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on tvOS 11 or newer}}}
+// expected-note@-10 {{if you supply your own aligned allocation functions}}
+#elif defined(WATCHOS)
+// expected-error@-12 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on watchOS 4 or newer}}}
+// expected-note@-13 {{if you supply your own aligned allocation functions}}
+#else
+// expected-error@-15 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on macOS 10.13 or newer}}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+#endif
+#endif
+
+void testExplicitOperatorNewDelete() {
+  void *p = operator new(128);
+  operator delete(p);
+  p = operator new[](128);
+  operator delete[](p);
+  p = __builtin_operator_new(128);
+  __builtin_operator_delete(p);
+}
+
+void testExplicitOperatorNewDeleteOveraligned() {
+  void *p = operator new(128, (std::align_val_t)64);
+  operator delete(p, (std::align_val_t)64);
+  p = operator new[](128, (std::align_val_t)64);
+  operator delete[](p, (std::align_val_t)64);
+  p = __builtin_operator_new(128, (std::align_val_t)64);
+  __builtin_operator_delete(p, (std::align_val_t)64);
+}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+// expected-error@-11 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-12 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-13 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-14 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-15 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-17 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-18 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-19 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-20 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-21 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-22 {{if you supply your own aligned allocation functions}}
+#endif
+
 void *operator new(std::size_t __sz, std::align_val_t) {
   static char array[256];
   return &array;
Index: test/CXX/drs/dr2xx.cpp
===
--- test/CXX/drs/dr2xx.cpp
+++ test/CXX/drs/dr2xx.cpp
@@ -718,7 +718,7 @@
 A() {}
   };
 
-  // FIXME: These are ill-formed, with a required diagnostic, for the same
+  // FIXME: This is ill-formed, with a required diagnostic, for the same
   // reason.
   struct B {
 inline void operator delete(void*) __attribute__((unused));
@@ -726,7 +726,7 @@
   };
   struct C {
 inline void operator delete(void*) __attribute__((unused));
-virtual ~C() {}
+virtual ~C() {} // expected-warning {{'operator delete' was marked unused but was used}}
   };
 
   struct D {

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a subscriber: donat.nagy.
Szelethus added a comment.

I see your point, but here's why I think it isn't a bug: I like to see macros 
as `constexpr` variables, and if I used those instead, I personally wouldn't 
like to get a warning just because they have the same value. In C, silencing 
such a warning isn't even really possible.

Another interesting thought, @donat.nagy's check works by comparing actual 
nodes of the AST, while this one would work with `Lexer`, but they almost want 
to do the same thing, the only difference is that the first checks whether two 
pieces of code are equivalent, and the second checks whether they are the same. 
Maybe it'd be worth to extract the logic into a simple `areStmtsEqual(const 
Stmt *S1, const Stmt *S2, bool ShouldCompareLexically = false)` function, that 
would do AST based comparison if the last param is set to false, and would use 
`Lexer` if set to true. After that, we could just add command line options to 
both of these checks, to leave it up to the user to choose in between them. 
Maybe folks who suffer from heavily macro-infested code would rather bear the 
obvious performance hit `Lexer` causes for little more precise warnings, and 
(for example) users of C++11 (because there are few excuses not to prefer 
`constexpr` there) could run in AST mode.

But I'm just thinking aloud really.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55125/new/

https://reviews.llvm.org/D55125



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: rsmith, chandlerc.
Herald added a subscriber: arphaman.

This warning, Wexperimental-driver-option, is on by default, exempt
from -Werror, and may be disabled.

Additionally, change the documentation to note that these options are
not intended for common usage.


https://reviews.llvm.org/D55150

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3419,6 +3419,11 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setSuppressAfterFatalError(false);
 
+  // Suppress driver warning for use of -Xclang, since we add it ourselves.
+  Diags->setSeverityForGroup(diag::Flavor::WarningOrError,
+ "experimental-driver-option",
+ diag::Severity::Ignored, SourceLocation());
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4994,6 +4994,11 @@
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
+  if (Args.hasArg(options::OPT_Xclang))
+D.Diag(diag::warn_drv_xclang_option);
+  if (Args.hasArg(options::OPT_mllvm))
+D.Diag(diag::warn_drv_mllvm_option);
+
   // -finclude-default-header flag is for preprocessor,
   // do not pass it to other cc1 commands when save-temps is enabled
   if (C.getDriver().isSaveTempsEnabled() &&
@@ -5926,6 +5931,8 @@
   CollectArgsForIntegratedAssembler(C, Args, CmdArgs,
 getToolChain().getDriver());
 
+  if (Args.hasArg(options::OPT_mllvm))
+D.Diag(diag::warn_drv_mllvm_option);
   Args.AddAllArgs(CmdArgs, options::OPT_mllvm);
 
   assert(Output.isFilename() && "Unexpected lipo output.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -466,7 +466,7 @@
   HelpText<"Pass  to the assembler">, MetaVarName<"">,
   Group;
 def Xclang : Separate<["-"], "Xclang">,
-  HelpText<"Pass  to the clang compiler">, MetaVarName<"">,
+  HelpText<"Pass  to the clang cc1 frontend. For experimental use only">, MetaVarName<"">,
   Flags<[DriverOption, CoreOption]>, Group;
 def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">,
   HelpText<"Pass  to fatbinary invocation">, MetaVarName<"">;
@@ -2002,7 +2002,7 @@
 def mlinker_version_EQ : Joined<["-"], "mlinker-version=">,
   Flags<[DriverOption]>;
 def mllvm : Separate<["-"], "mllvm">, Flags<[CC1Option,CC1AsOption,CoreOption]>,
-  HelpText<"Additional arguments to forward to LLVM's option processing">;
+  HelpText<"Additional arguments to forward to LLVM's option processing. For experimental use only">;
 def mmacosx_version_min_EQ : Joined<["-"], "mmacosx-version-min=">,
   Group, HelpText<"Set Mac OS X deployment target">;
 def mmacos_version_min_EQ : Joined<["-"], "mmacos-version-min=">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1039,3 +1039,7 @@
 // A warning group specifically for warnings related to function
 // multiversioning.
 def FunctionMultiVersioning : DiagGroup<"function-multiversion">;
+
+// Warning for command-line options that are not a supported part of the clang command-line interface.
+// Warnings in this group should be on by default, and exempt from -Werror.
+def ExperimentalDriverOption : DiagGroup<"experimental-driver-option">;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -405,4 +405,14 @@
 def warn_drv_moutline_unsupported_opt : Warning<
   "The '%0' architecture does not support -moutline; flag ignored">,
   InGroup;
+
+def warn_drv_xclang_option : Warning<
+  "-Xclang options are for experimental use only; future compatibility may not be preserved">,
+  InGroup, DefaultWarnNoWerror;
+
+def warn_drv_mllvm_option : Warning<
+  "-mllvm options are for experimental use only; future compatibility may not be preserved">,
+  InGroup, DefaultWarnNoWerror;
+
 }
+
Index: clang/docs/UsersManual.rst
===
--- clang/docs/User

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

> Would you be okay with landing this fix and if we get any further reports for 
> Objective-C++ sources then we can suppress it in all C++/Objective-C++ 
> sources? I think there is enough value to enforcing the naming conventions on 
> non-namespaced C functions in Objective-C++ to justify a simple followup fix. 
> If other issues are reported after this then I also agree that enforcement in 
> Objective-C++ sources may incur more overhead than it's worth.

I'm not against it, but we've already disabled the majority of Objective-C 
checks for Objective-C++ code, so I don't think this one should apply either.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

PTAL,  it's a single line change, I just need someone to confirm that I'm not 
mistaken :)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55066/new/

https://reviews.llvm.org/D55066



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348044 - [analyzer] Deleting unnecessary test file

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Fri Nov 30 14:32:17 2018
New Revision: 348044

URL: http://llvm.org/viewvc/llvm-project?rev=348044&view=rev
Log:
[analyzer] Deleting unnecessary test file

That I really should've done in rC348031.

Removed:
cfe/trunk/test/Analysis/analyzer-config.cpp

Removed: cfe/trunk/test/Analysis/analyzer-config.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=348043&view=auto
==
--- cfe/trunk/test/Analysis/analyzer-config.cpp (original)
+++ cfe/trunk/test/Analysis/analyzer-config.cpp (removed)
@@ -1,54 +0,0 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 
2>&1
-// RUN: FileCheck --input-file=%t %s --match-full-lines
-
-// CHECK: [config]
-// CHECK-NEXT: aggressive-binary-operation-simplification = false
-// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
-// CHECK-NEXT: c++-allocator-inlining = true
-// CHECK-NEXT: c++-container-inlining = false
-// CHECK-NEXT: c++-inlining = destructors
-// CHECK-NEXT: c++-shared_ptr-inlining = false
-// CHECK-NEXT: c++-stdlib-inlining = true
-// CHECK-NEXT: c++-temp-dtor-inlining = true
-// CHECK-NEXT: c++-template-inlining = true
-// CHECK-NEXT: cfg-conditional-static-initializers = true
-// CHECK-NEXT: cfg-implicit-dtors = true
-// CHECK-NEXT: cfg-lifetime = false
-// CHECK-NEXT: cfg-loopexit = false
-// CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-scopes = false
-// CHECK-NEXT: cfg-temporary-dtors = true
-// CHECK-NEXT: crosscheck-with-z3 = false
-// CHECK-NEXT: ctu-dir = ""
-// CHECK-NEXT: ctu-index-name = externalFnMap.txt
-// CHECK-NEXT: eagerly-assume = true
-// CHECK-NEXT: elide-constructors = true
-// CHECK-NEXT:expand-macros = false
-// CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
-// CHECK-NEXT: exploration_strategy = unexplored_first_queue
-// CHECK-NEXT: faux-bodies = true
-// CHECK-NEXT: graph-trim-interval = 1000
-// CHECK-NEXT: inline-lambdas = true
-// CHECK-NEXT: ipa = dynamic-bifurcate
-// CHECK-NEXT: ipa-always-inline-size = 3
-// CHECK-NEXT: max-inlinable-size = 100
-// CHECK-NEXT: max-nodes = 225000
-// CHECK-NEXT: max-symbol-complexity = 35
-// CHECK-NEXT: max-times-inline-large = 32
-// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
-// CHECK-NEXT: mode = deep
-// CHECK-NEXT: model-path = ""
-// CHECK-NEXT: notes-as-events = false
-// CHECK-NEXT: objc-inlining = true
-// CHECK-NEXT: prune-paths = true
-// CHECK-NEXT: region-store-small-struct-limit = 2
-// CHECK-NEXT: report-in-main-source-file = false
-// CHECK-NEXT: serialize-stats = false
-// CHECK-NEXT: stable-report-filename = false
-// CHECK-NEXT: suppress-c++-stdlib = true
-// CHECK-NEXT: suppress-inlined-defensive-checks = true
-// CHECK-NEXT: suppress-null-return-paths = true
-// CHECK-NEXT: unroll-loops = false
-// CHECK-NEXT: widen-loops = false
-// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 48


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In D55101#1314716 , @benhamilton wrote:

> IMHO we should just disable this check entirely for C++ files (including 
> Objective-C++).
>
> Since Objective-C++ files will have a mix of the Google Objective-C and 
> Google C++ styles, I think there will be too many places where we'll hit 
> conflicts like this (there are likely more).


Would you be okay with landing this fix and if we get any further reports for 
Objective-C++ sources then we can suppress it in all C++/Objective-C++ sources? 
I think there is enough value to enforcing the naming conventions on 
non-namespaced C functions in Objective-C++ to justify a simple followup fix. 
If other issues are reported after this then I also agree that enforcement in 
Objective-C++ sources may incur more overhead than it's worth.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 176206.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Re-formatted google-objc-function-naming.mm to LLVM style.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.mm


Index: test/clang-tidy/google-objc-function-naming.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),


Index: test/clang-tidy/google-objc-function-naming.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D53812: [Analyzer] Iterator Checker - Forbid increments past the begin() and decrements past the end() of containers

[Artem Dergachev via cfe-commits]

I suspect that the point is that previously we believed that 
decrementing below begin and than *incrementing* it back to begin() 
would yield begin(), but now we disallow this "incrementing back" 
behavior and warn immediately on decrement because in fact it's already UB.


Though, yeah, the title is wonky.

On 11/30/18 2:19 PM, Roman Lebedev wrote:

""Re: [PATCH] D53812: [Analyzer] Iterator Checker - Forbid increments
past the begin() and decrements past the end() of containers"
Is it be or that reads backwards?
Why can't you increment past the begin()? Can you decrement past the
begin() instead?
And the opposite for the end().

On Sat, Dec 1, 2018 at 1:17 AM Artem Dergachev via Phabricator via
cfe-commits  wrote:

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Makes perfect sense to me!




Comment at: test/Analysis/iterator-range.cpp:190
+  auto i0 = L.begin();
+  --i0;  // expected-warning{{Iterator accessed outside of its range}}
+}

I guess we'll have to come up with a separate warning text for this case, as 
there's no access happening through the iterator.

Regardless of how the final message would look, i suggest adding logic for 
having a different message now, so that we didn't forget about this case later.


CHANGES SINCE LAST ACTION
   https://reviews.llvm.org/D53812/new/

https://reviews.llvm.org/D53812



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/include/clang/Basic/FixedPoint.h:67
+  FixedPointSemantics
+  getCommonSemantics(const FixedPointSemantics &Other) const;
+

rjmccall wrote:
> leonardchan wrote:
> > rjmccall wrote:
> > > Actually representing the fully-precise value is operation-specific; 
> > > you'd need a different calculation for at least addition and 
> > > multiplication, and maybe also subtraction and (maybe?) division.  Is 
> > > this what you actually want?
> > For addition and subtraction, I believe we just need to extend and shift to 
> > a type that is large enough to hold the larger scale and integral bits, 
> > then we can do a normal add/sub. I think the relational operations can use 
> > this also since we would just need to resize and align scales for them.
> > 
> > For multiplication, the intrinsics we will use also assume the scale for 
> > both operands are the same, which I believe will also require finding 
> > semantics large enough to hold the larger scale and integral bits.
> > 
> > ```
> > declare iN @llvm.smul.fix(iN %L, %iN R, i32 %Scale)
> > ```
> > 
> > Division is similar since the saturating and non-saturating intrinsics 
> > assume both operands have the same scale.
> > 
> > ```
> > declare iN @llvm.sdiv.fix(iN %L, %iN R, i32 %Scale)
> > ```
> > 
> > In each of these cases, I believe the common semantics just needs to be 
> > large enough to hold the scale and largest representable value, which is 
> > what this method does.
> Okay, so I believe what you're saying is that `getCommonSemantics` is defined 
> as returning a semantics that is capable of precisely representing both input 
> values, not one that's capable of precisely representing the result of the 
> computation.  The documentation could be clearer about that.
Yup. Updated the documentation.



Comment at: clang/lib/Sema/SemaExpr.cpp:1304
+RHSTy = ResultTy;
+  }
+

rjmccall wrote:
> ebevhan wrote:
> > rjmccall wrote:
> > > Hmm.  So adding a signed integer to an unsigned fixed-point type always 
> > > converts the integer to unsigned?  That's a little weird, but only 
> > > because the fixed-point rule seems to be the other way.  Anyway, I assume 
> > > it's not a bug in the spec.
> > `handleFixedPointConversion` only calculates the result type of the 
> > expression, not the calculation type. The final result type of the 
> > operation will be the unsigned fixed-point type, but the calculation itself 
> > will be done in a signed type with enough precision to represent both the 
> > signed integer and the unsigned fixed-point type. 
> > 
> > Though, if the result ends up being negative, the final result is undefined 
> > unless the type is saturating. I don't think there is a test for the 
> > saturating case (signed integer + unsigned saturating fixed-point) in the 
> > SaturatedAddition tests.
> > `handleFixedPointConversion` only calculates the result type of the 
> > expression, not the calculation type.
> 
> Right, I understand that, but the result type of the expression obviously 
> impacts the expressive range of the result, since you can end up with a 
> negative value.
> 
> Hmm.  With that said, if the general principle is to perform the operation 
> with full precision on the original operand values, why are unsigned 
> fixed-point operands coerced to their corresponding signed types *before* the 
> operation?  This is precision-limiting if the unsigned representation doesn't 
> use a padding bit.  That seems like a bug in the spec.
> Hmm. With that said, if the general principle is to perform the operation 
> with full precision on the original operand values, why are unsigned 
> fixed-point operands coerced to their corresponding signed types *before* the 
> operation? This is precision-limiting if the unsigned representation doesn't 
> use a padding bit. That seems like a bug in the spec.

Possibly. When the standard mentions converting from signed to unsigned fixed 
point, the only requirement involved is conservation of magnitude (the number 
of integral bits excluding the sign)

```
when signed and unsigned fixed-point types are mixed, the unsigned type is 
converted to the corresponding signed type, and this should go without loss of 
magnitude
```

This is just speculation, but I'm under the impression that not as much 
"attention" was given for unsigned types as for signed types since "`In the 
embedded processor world, support for unsigned fixed-point data types is rare; 
normally only signed fixed-point data types are supported`", but was kept 
anyway since unsigned types are used a lot.

```
However, to disallow unsigned fixed-point arithmetic from programming languages 
(in general, and from C in particular) based on this observation, seems overly 
restrictive.
```

I also imagine that if the programmer needs more precision, the correct 
approach would be to cast up to a type with a higher scale. The

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 176204.
leonardchan marked 6 inline comments as done.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53738/new/

https://reviews.llvm.org/D53738

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Frontend/fixed_point_add.c
  clang/test/Frontend/fixed_point_add_ast.c
  clang/test/Frontend/fixed_point_conversions.c

Index: clang/test/Frontend/fixed_point_conversions.c
===
--- clang/test/Frontend/fixed_point_conversions.c
+++ clang/test/Frontend/fixed_point_conversions.c
@@ -214,19 +214,17 @@
   // Only get overflow checking if signed fract to unsigned accum
   sat_ua = sat_sf;
   // DEFAULT:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i17
-  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i17 [[FRACT_EXT]], 9
-  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i17 [[ACCUM]], 0
-  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i17 0, i17 [[ACCUM]]
-  // DEFAULT-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i17 [[RESULT]] to i32
-  // DEFAULT-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 9
+  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // DEFAULT-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
   // SAME:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i16
-  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i16 [[FRACT_EXT]], 8
-  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i16 [[ACCUM]], 0
-  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i16 0, i16 [[ACCUM]]
-  // SAME-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i16 [[RESULT]] to i32
-  // SAME-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 8
+  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // SAME-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
 }
 
 void TestFixedPointCastBetFractAccum() {
Index: clang/test/Frontend/fixed_point_add_ast.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_add_ast.c
@@ -0,0 +1,337 @@
+// RUN: %clang_cc1 -x c -ffixed-point -ast-dump %s | FileCheck %s --strict-whitespace
+
+void SignedAdditions() {
+  // CHECK-LABEL: SignedAdditions
+  short _Accum sa;
+  _Accum a;
+  long _Accum la;
+  unsigned short _Accum usa;
+  unsigned _Accum ua;
+  unsigned long _Accum ula;
+
+  short _Fract sf;
+  _Fract f;
+  long _Fract lf;
+  unsigned short _Fract usf;
+  unsigned _Fract uf;
+  unsigned long _Fract ulf;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'short _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  sa = sa + sa;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  a = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'long _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'long _Accum' {{.*}} 'la' 'long _Accum'
+  la = sa + la;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT:`-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:  |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:  | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:  `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT:`-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  la = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHEC

Re: [PATCH] D53812: [Analyzer] Iterator Checker - Forbid increments past the begin() and decrements past the end() of containers

[Roman Lebedev via cfe-commits]

""Re: [PATCH] D53812: [Analyzer] Iterator Checker - Forbid increments
past the begin() and decrements past the end() of containers"
Is it be or that reads backwards?
Why can't you increment past the begin()? Can you decrement past the
begin() instead?
And the opposite for the end().

On Sat, Dec 1, 2018 at 1:17 AM Artem Dergachev via Phabricator via
cfe-commits  wrote:
>
> NoQ accepted this revision.
> NoQ added a comment.
> This revision is now accepted and ready to land.
>
> Makes perfect sense to me!
>
>
>
> 
> Comment at: test/Analysis/iterator-range.cpp:190
> +  auto i0 = L.begin();
> +  --i0;  // expected-warning{{Iterator accessed outside of its range}}
> +}
> 
> I guess we'll have to come up with a separate warning text for this case, as 
> there's no access happening through the iterator.
>
> Regardless of how the final message would look, i suggest adding logic for 
> having a different message now, so that we didn't forget about this case 
> later.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D53812/new/
>
> https://reviews.llvm.org/D53812
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53812: [Analyzer] Iterator Checker - Forbid increments past the begin() and decrements past the end() of containers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Makes perfect sense to me!




Comment at: test/Analysis/iterator-range.cpp:190
+  auto i0 = L.begin();
+  --i0;  // expected-warning{{Iterator accessed outside of its range}}
+}

I guess we'll have to come up with a separate warning text for this case, as 
there's no access happening through the iterator.

Regardless of how the final message would look, i suggest adding logic for 
having a different message now, so that we didn't forget about this case later.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53812/new/

https://reviews.llvm.org/D53812



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53754: [Analyzer] Skip symbolic regions based on conjured symbols in comparison of the containers of iterators

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

What makes you think that conjured symbols are special?

Doesn't the same problem appear in the following example?:

  void foo(std::vector &v1, std::vector &v2) {
v2.erase(v1.cbegin());
  }

In this example if `foo()` is analyzed as a top level function, the respective 
symbols would be of `SymbolRegionValue` kind. It is also easy to come up with a 
test case that involves `SymbolDerived`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53754/new/

https://reviews.llvm.org/D53754



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please close PR30233 after committing patch.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54757/new/

https://reviews.llvm.org/D54757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53701: [Analyzer] Record and process comparison of symbols instead of iterator positions in interator checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added a subscriber: gamesh411.

In D53701#1288682 , 
@baloghadamsoftware wrote:

> I am almost sure it is implossible. It is not only about iterator-adapters in 
> the classical sense but also any iterator that internally deals with other 
> iterators.
>
> For example I created an iterator in the past that iterates over the elements 
> of a list of lists. This iterator contained two iterators, one for the outer 
> and one for the inner list. In this case determining whether two iterators 
> are the same is non-trivial: for all in-range iterators both the outer and 
> the inner iterators must be the same. However if the outer iterator is the 
> past-end iterator of the outer list then the inner iterator is irrelevant. 
> Thus the comparison operator of such iterator must check first whether any of 
> the outer iterators is the past-end iterator and only compare the inner 
> iterator none of them is. If both outer iterators are the past-end iterator 
> then they are equal regardless of the inner iterators.
>
> Another example could be an iterator that somehow merges two lists internally 
> using two different iterators. In this case only one of the inner iterators 
> is relevant when comparing two merging iterators.
>
> So by dropping the inlining we lose some intrinsic knowledge of the analyzed 
> code which leads the checker to wrong assumptions.


The nested iterator thing looks easy to detect heuristically. Just have a look 
if any of the fields within the object are of iterator type.

I think it's worth a try to do a total evalCall at first, and then disable 
evalCall (together with the attempt to model the iterator position) in an 
incrementally growing blacklist of cases (1. iterator adaptors, 2. ) as we 
encounter problems. This essentially becomes part of the logic that decides 
whether an object is an iterator. Eg., if it's more like an adaptor than an 
actual iterator, let's treat it as if it's not an iterator, but inline instead, 
and hope that we model the underlying iterators correctly via evalCall.

This does look hacky, but it does look less hacky than trying to align two 
models of the iterator position. Or is it actually necessary to maintain the 
two models of the iterator position in order to avoid these false positives? If 
so, could you give an example?

> When I first started working on Clang Static Analyzer Anna told me the 
> `-analyzer-eagerly-assume` should be the default.

Which is why i suggest a similar behavior here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53701/new/

https://reviews.llvm.org/D53701



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Paul Robinson via Phabricator via cfe-commits]

probinson accepted this revision.
probinson added a comment.
This revision is now accepted and ready to land.

LGTM aside from a grump about the test, which is totally up to you.




Comment at: test/CodeGenCXX/debug-prefix-map-lambda.cpp:7
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"

I've never liked tests that hard-code the test file name. FileCheck lets you 
define variables on the RUN line, is there a lit substitution that will return 
just the basename?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348041 - Add myself as code owner for OpenBSD driver

[Brad Smith via cfe-commits]

Author: brad
Date: Fri Nov 30 13:42:34 2018
New Revision: 348041

URL: http://llvm.org/viewvc/llvm-project?rev=348041&view=rev
Log:
Add myself as code owner for OpenBSD driver

Modified:
cfe/trunk/CODE_OWNERS.TXT

Modified: cfe/trunk/CODE_OWNERS.TXT
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CODE_OWNERS.TXT?rev=348041&r1=348040&r2=348041&view=diff
==
--- cfe/trunk/CODE_OWNERS.TXT (original)
+++ cfe/trunk/CODE_OWNERS.TXT Fri Nov 30 13:42:34 2018
@@ -49,6 +49,10 @@ N: John McCall
 E: rjmcc...@apple.com
 D: Clang LLVM IR generation
 
+N: Brad Smith
+E: b...@comstyle.com
+D: OpenBSD driver
+
 N: Richard Smith
 E: rich...@metafoo.co.uk
 D: All parts of Clang not covered by someone else


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176194.
aprantl added a comment.

Only initialize `RemapFilePaths` when `DebugPrefixMap` is nonempty. Should be 
slightly faster.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137

Files:
  include/clang/AST/PrettyPrinter.h
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-prefix-map-lambda.cpp

Index: test/CodeGenCXX/debug-prefix-map-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-prefix-map-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   -fdebug-prefix-map=%S=/SOURCE_ROOT %s -emit-llvm -o - | FileCheck %s
+
+template  void b(T) {}
+void c() {
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
+  b([]{});
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -341,6 +341,9 @@
 
   void finalize();
 
+  /// Remap a path using the -fdebug-prefix-map.
+  std::string remapDIPath(StringRef Path) const;
+
   /// Register VLA size expression debug node with the qualified type.
   void registerVLASizeExpression(QualType Ty, llvm::Metadata *SizeExpr) {
 SizeExprCache[Ty] = SizeExpr;
@@ -528,9 +531,6 @@
   /// Create new compile unit.
   void CreateCompileUnit();
 
-  /// Remap a given path with the current debug prefix map
-  std::string remapDIPath(StringRef) const;
-
   /// Compute the file checksum debug info for input file ID.
   Optional
   computeChecksum(FileID FID, SmallString<32> &Checksum) const;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -234,6 +234,11 @@
   if (CGM.getCodeGenOpts().EmitCodeView)
 PP.MSVCFormatting = true;
 
+  // Apply -fdebug-prefix-map.
+  if (!DebugPrefixMap.empty()) {
+PP.RemapFilePaths = true;
+PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };
+  }
   return PP;
 }
 
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1157,9 +1157,13 @@
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -38,21 +38,20 @@
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(false), ConstantsAsWritten(false),
+SuppressImplicitBase(false), FullyQualifiedName(false),
+

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D55137#1315133 , @dblaikie wrote:

> Can't say I know much abouth the path remapping functionality - what it's 
> used for, where it's implemented in general, etc - so figure someone with 
> more of that knowledge might be best off reviewing this.


One common use-case for path remapping is a build system that builds on a 
different machine in some local directory, but wanting the debug info appear as 
if the source code were in a previously agreed-upon path so it can be debugged 
on another machine were the source code can be found in that same path.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348039 - Revert r347417 "Re-Reinstate 347294 with a fix for the failures."

[Fangrui Song via cfe-commits]

Author: maskray
Date: Fri Nov 30 13:26:09 2018
New Revision: 348039

URL: http://llvm.org/viewvc/llvm-project?rev=348039&view=rev
Log:
Revert r347417 "Re-Reinstate 347294 with a fix for the failures."

Kept the "indirect_builtin_constant_p" test case in 
test/SemaCXX/constant-expression-cxx1y.cpp
while we are investigating why the following snippet fails:

  extern char extern_var;
  struct { int a; } a = {__builtin_constant_p(extern_var)};

Removed:
cfe/trunk/test/CodeGen/builtin-constant-p.c
cfe/trunk/test/CodeGenCXX/builtin-constant-p.cpp
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
cfe/trunk/test/Analysis/builtin-functions.cpp
cfe/trunk/test/Sema/builtins.c
cfe/trunk/test/SemaCXX/compound-literal.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=348039&r1=348038&r2=348039&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Fri Nov 30 13:26:09 2018
@@ -583,8 +583,7 @@ public:
   /// this function returns true, it returns the folded constant in Result. If
   /// the expression is a glvalue, an lvalue-to-rvalue conversion will be
   /// applied.
-  bool EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx,
-bool InConstantContext = false) const;
+  bool EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx) const;
 
   /// EvaluateAsBooleanCondition - Return true if this is a constant
   /// which we can fold and convert to a boolean condition using
@@ -601,7 +600,7 @@ public:
 
   /// EvaluateAsInt - Return true if this is a constant which we can fold and
   /// convert to an integer, using any crazy technique that we want to.
-  bool EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
+  bool EvaluateAsInt(llvm::APSInt &Result, const ASTContext &Ctx,
  SideEffectsKind AllowSideEffects = SE_NoSideEffects) 
const;
 
   /// EvaluateAsFloat - Return true if this is a constant which we can fold and
@@ -902,15 +901,10 @@ public:
 
 /// ConstantExpr - An expression that occurs in a constant context.
 class ConstantExpr : public FullExpr {
+public:
   ConstantExpr(Expr *subexpr)
 : FullExpr(ConstantExprClass, subexpr) {}
 
-public:
-  static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
-assert(!isa(E));
-return new (Context) ConstantExpr(E);
-  }
-
   /// Build an empty constant expression wrapper.
   explicit ConstantExpr(EmptyShell Empty)
 : FullExpr(ConstantExprClass, Empty) {}
@@ -3093,8 +3087,8 @@ inline Expr *Expr::IgnoreImpCasts() {
   while (true)
 if (ImplicitCastExpr *ice = dyn_cast(e))
   e = ice->getSubExpr();
-else if (FullExpr *fe = dyn_cast(e))
-  e = fe->getSubExpr();
+else if (ConstantExpr *ce = dyn_cast(e))
+  e = ce->getSubExpr();
 else
   break;
   return e;

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=348039&r1=348038&r2=348039&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Nov 30 13:26:09 2018
@@ -6388,7 +6388,7 @@ ExpectedStmt ASTNodeImporter::VisitConst
   Expr *ToSubExpr;
   std::tie(ToSubExpr) = *Imp;
 
-  return ConstantExpr::Create(Importer.getToContext(), ToSubExpr);
+  return new (Importer.getToContext()) ConstantExpr(ToSubExpr);
 }
 
 ExpectedStmt AST

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

[Umann Kristóf via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Szelethus marked an inline comment as done.
Closed by commit rC348038: [analyzer] Emit an error for invalid 
-analyzer-config inputs (authored by Szelethus, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53280?vs=174495&id=176191#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53280/new/

https://reviews.llvm.org/D53280

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Analysis/invalid-analyzer-config-value.c

Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2368,6 +2368,9 @@
   // Treat blocks as analysis entry points.
   CmdArgs.push_back("-analyzer-opt-analyze-nested-blocks");
 
+  // Enable compatilibily mode to avoid analyzer-config related errors.
+  CmdArgs.push_back("-analyzer-config-compatibility-mode=true");
+
   // Add default argument set.
   if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {
 CmdArgs.push_back("-analyzer-checker=core");
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -181,8 +181,10 @@
   }
 }
 
+// Parse the Static Analyzer configuration. If \p Diags is set to nullptr,
+// it won't verify the input.
 static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,
- DiagnosticsEngine &Diags);
+ DiagnosticsEngine *Diags);
 
 static void getAllNoBuiltinFuncValues(ArgList &Args,
   std::vector &Funcs) {
@@ -284,6 +286,12 @@
   Opts.ShowCheckerHelp = Args.hasArg(OPT_analyzer_checker_help);
   Opts.ShowConfigOptionsList = Args.hasArg(OPT_analyzer_config_help);
   Opts.ShowEnabledCheckerList = Args.hasArg(OPT_analyzer_list_enabled_checkers);
+  Opts.ShouldEmitErrorsOnInvalidConfigValue =
+  /* negated */!llvm::StringSwitch(
+   Args.getLastArgValue(OPT_analyzer_config_compatibility_mode))
+.Case("true", true)
+.Case("false", false)
+.Default(false);
   Opts.DisableAllChecks = Args.hasArg(OPT_analyzer_disable_all_checks);
 
   Opts.visualizeExplodedGraphWithGraphViz =
@@ -320,7 +328,7 @@
 
   // Go through the analyzer configuration options.
   for (const auto *A : Args.filtered(OPT_analyzer_config)) {
-A->claim();
+
 // We can have a list of comma separated config names, e.g:
 // '-analyzer-config key1=val1,key2=val2'
 StringRef configList = A->getValue();
@@ -342,11 +350,24 @@
 Success = false;
 break;
   }
+
+  // TODO: Check checker options too, possibly in CheckerRegistry.
+  // Leave unknown non-checker configs unclaimed.
+  if (!key.contains(":") && Opts.isUnknownAnalyzerConfig(key)) {
+if (Opts.ShouldEmitErrorsOnInvalidConfigValue)
+  Diags.Report(diag::err_analyzer_config_unknown) << key;
+continue;
+  }
+
+  A->claim();
   Opts.Config[key] = val;
 }
   }
 
-  parseAnalyzerConfigs(Opts, Diags);
+  if (Opts.ShouldEmitErrorsOnInvalidConfigValue)
+parseAnalyzerConfigs(Opts, &Diags);
+  else
+parseAnalyzerConfigs(Opts, nullptr);
 
   llvm::raw_string_ostream os(Opts.FullCompilerInvocation);
   for (unsigned i = 0; i < Args.getNumInputArgStrings(); ++i) {
@@ -365,56 +386,82 @@
 }
 
 static void initOption(AnalyzerOptions::ConfigTable &Config,
+   DiagnosticsEngine *Diags,
StringRef &OptionField, StringRef Name,
StringRef DefaultVal) {
+  // String options may be known to invalid (e.g. if the expected string is a
+  // file name, but the file does not exist), those will have to be checked in
+  // parseConfigs.
   OptionField = getStringOption(Config, Name, DefaultVal);
 }
 
 static void initOption(AnalyzerOptions::ConfigTable &Config,
+   DiagnosticsEngine *Diags,
bool &OptionField, StringRef Name, bool DefaultVal) {
-  // FIXME: We should emit a warning here if the value is something other than
-  // "true", "false", or the empty string (meaning the default value),
-  // but the AnalyzerOptions doesn't have access to a diagnostic engine.
-  OptionField = llvm::StringSwitch(getStringOption(Config, Name,
-   (DefaultVal ? "true" : "false")))
+  auto PossiblyInvalidVal = llvm::StringSwitch>(
+ getStringOption(Config, Name, (DefaultVal ? "true" : "false")))
   .Case("true", true)
   .Case("false", false)
-  .Default(DefaultVal);
+  .Default(None);
+
+  if (!PossiblyInvalidVal) {

r348038 - [analyzer] Emit an error for invalid -analyzer-config inputs

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Fri Nov 30 13:24:31 2018
New Revision: 348038

URL: http://llvm.org/viewvc/llvm-project?rev=348038&view=rev
Log:
[analyzer] Emit an error for invalid -analyzer-config inputs

Differential Revision: https://reviews.llvm.org/D53280

Added:
cfe/trunk/test/Analysis/invalid-analyzer-config-value.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=348038&r1=348037&r2=348038&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Nov 30 13:24:31 
2018
@@ -299,6 +299,9 @@ def err_analyzer_config_no_value : Error
   "analyzer-config option '%0' has a key but no value">;
 def err_analyzer_config_multiple_values : Error<
   "analyzer-config option '%0' should contain only one '='">;
+def err_analyzer_config_invalid_input : Error<
+  "invalid input for analyzer-config option '%0', that expects %1 value">;
+def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">;
 
 def err_drv_invalid_hvx_length : Error<
   "-mhvx-length is not supported without a -mhvx/-mhvx= flag">;

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=348038&r1=348037&r2=348038&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Fri Nov 30 13:24:31 2018
@@ -138,6 +138,12 @@ def analyzer_list_enabled_checkers : Fla
 def analyzer_config : Separate<["-"], "analyzer-config">,
   HelpText<"Choose analyzer options to enable">;
 
+def analyzer_config_compatibility_mode : Separate<["-"], 
"analyzer-config-compatibility-mode">,
+  HelpText<"Don't emit errors on invalid analyzer-config inputs">;
+
+def analyzer_config_compatibility_mode_EQ : Joined<["-"], 
"analyzer-config-compatibility-mode=">,
+  Alias;
+
 
//===--===//
 // Migrator Options
 
//===--===//

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=348038&r1=348037&r2=348038&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Fri Nov 30 
13:24:31 2018
@@ -200,6 +200,7 @@ public:
   unsigned ShowCheckerHelp : 1;
   unsigned ShowEnabledCheckerList : 1;
   unsigned ShowConfigOptionsList : 1;
+  unsigned ShouldEmitErrorsOnInvalidConfigValue : 1;
   unsigned AnalyzeAll : 1;
   unsigned AnalyzerDisplayProgress : 1;
   unsigned AnalyzeNestedBlocks : 1;
@@ -222,6 +223,7 @@ public:
   /// The mode of function selection used during inlining.
   AnalysisInliningMode InliningMode = NoRedundancy;
 
+  // Create a field for each -analyzer-config option.
 #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,
\
  SHALLOW_VAL, DEEP_VAL)
\
   ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
@@ -233,13 +235,39 @@ public:
 #undef ANALYZER_OPTION
 #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
 
+  // Create an array of all -analyzer-config command line options. Sort it in
+  // the constructor.
+  std::vector AnalyzerConfigCmdFlags = {
+#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,
\
+ SHALLOW_VAL, DEEP_VAL)
\
+  ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
+
+#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL)
\
+CMDFLAG,
+
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
+#undef ANALYZER_OPTION
+#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
+  };
+
+  bool isUnknownAnalyzerConfig(StringRef Name) const {
+
+assert(std::is_sorted(AnalyzerConfigCmdFlags.begin(),
+  AnalyzerConfigCmdFlags.end()));
+
+return !std::binary_search(AnalyzerConfigCmdFlags.begin(),
+   AnalyzerConfigCmdFlags.end(), Name);
+  }
+
   AnalyzerOptions()
   : DisableAllChecks(false), ShowCheckerHelp(false),
 ShowEnabledCheckerList(false), ShowConfigOptionsList(false),
 AnalyzeAll(fa

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus marked 6 inline comments as done.
Szelethus added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:367
 
-  parseAnalyzerConfigs(Opts, Diags);
+  if (Opts.ShouldEmitErrorsOnInvalidConfigValue)
+parseAnalyzerConfigs(Opts, &Diags);

xazax.hun wrote:
> Do we actually need the branching here? It would be perfectly fine to always 
> pass a pointer to `Diags` but sometimes just ignore it.
I'm not sure what would be the point of that, if we don't branch here, we'll 
have to somewhere else, and I'm not sure whether polluting 
`parseAnalyzerConfigs` with that would help on readability.

Or, if you mean that I should just check 
`Opts.ShouldEmitErrorsOnInvalidConfigValue` inside there, that could work, but 
I think it makes more sense that if we don't to validate configs, don't even 
supply the tool for it. Less room for error.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53280/new/

https://reviews.llvm.org/D53280



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

So far everything looks fine, but I have to rerun a couple of things due to 
infrastructural issues. I should have the final results next Monday.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54630/new/

https://reviews.llvm.org/D54630



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348037 - [ExprConstant] Try fixing __builtin_constant_p after D54355 (rC347417)

[Fangrui Song via cfe-commits]

Author: maskray
Date: Fri Nov 30 13:15:41 2018
New Revision: 348037

URL: http://llvm.org/viewvc/llvm-project?rev=348037&view=rev
Log:
[ExprConstant] Try fixing __builtin_constant_p after D54355 (rC347417)

Summary:
Reinstate the original behavior (Success(false, E)) before D54355 when this 
branch is
taken. This fixes spurious error of the following snippet:

  extern char extern_var;
  struct { int a; } a = {__builtin_constant_p(extern_var)};

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=348037&r1=348036&r2=348037&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Nov 30 13:15:41 2018
@@ -8199,7 +8199,6 @@ bool IntExprEvaluator::VisitBuiltinCallE
   // We can delay calculation of __builtin_constant_p until after
   // inlining. Note: This diagnostic won't be shown to the user.
   Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
-  return false;
 }
 return Success(false, E);
   }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348033 - Adding tests for -ast-dump; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Nov 30 12:55:26 2018
New Revision: 348033

URL: http://llvm.org/viewvc/llvm-project?rev=348033&view=rev
Log:
Adding tests for -ast-dump; NFC.

This adds tests for struct and union declarations in C. It also points out a 
bug when dumping anonymous record types -- they are sometimes reported as being 
contained by something of the wrong tag type. e.g., an anonymous struct inside 
of a union named X reports the anonymous struct as being inside of 'struct X' 
rather than 'union X'.

Added:
cfe/trunk/test/AST/ast-dump-records.c

Added: cfe/trunk/test/AST/ast-dump-records.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-records.c?rev=348033&view=auto
==
--- cfe/trunk/test/AST/ast-dump-records.c (added)
+++ cfe/trunk/test/AST/ast-dump-records.c Fri Nov 30 12:55:26 2018
@@ -0,0 +1,150 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck 
-strict-whitespace %s
+
+struct A;
+// CHECK: RecordDecl 0x{{[^ ]*}} <{{.*}}:1, col:8> col:8 struct A
+
+struct B;
+// CHECK: RecordDecl 0x{{[^ ]*}}  col:8 struct B
+
+struct A {
+  // CHECK: RecordDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}}  line:[[@LINE-1]]:8 struct A definition
+  int a;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 a 
'int'
+  int b, c;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 b 
'int'
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:10 c 'int'
+  int d : 12;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 d 
'int'
+  // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 12
+  int : 0;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:3 'int'
+  // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 0
+  int e : 10;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 e 
'int'
+  // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  struct B *f;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:13 f 
'struct B *'
+};
+
+struct C {
+  // CHECK: RecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct C definition
+  struct {
+// CHECK-NEXT: RecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:3 struct definition
+int a;
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:9 a 
'int'
+  } b;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:5 b 'struct (anonymous struct at 
{{.*}}:[[@LINE-5]]:3)':'struct C::(anonymous at {{.*}}:[[@LINE-5]]:3)'
+
+  union {
+// CHECK-NEXT: RecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:3 union definition
+int c;
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:9 c 
'int'
+float d;
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:11 d 
'float'
+  };
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:3 implicit 
'union C::(anonymous at {{.*}}:[[@LINE-7]]:3)'
+  // CHECK-NEXT: IndirectFieldDecl 0x{{[^ ]*}}  col:9 
implicit c 'int'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} '' 'union C::(anonymous at 
{{.*}}:[[@LINE-9]]:3)'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} 'c' 'int'
+  // CHECK-NEXT: IndirectFieldDecl 0x{{[^ ]*}}  col:11 
implicit d 'float'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} '' 'union C::(anonymous at 
{{.*}}:[[@LINE-12]]:3)'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} 'd' 'float'
+
+  struct {
+// CHECK-NEXT: RecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:3 struct definition
+int e, f;
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:9 e 
'int'
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:12 f 'int'
+  };
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:3 implicit 
'struct C::(anonymous at {{.*}}:[[@LINE-6]]:3)'
+  // CHECK-NEXT: IndirectFieldDecl 0x{{[^ ]*}}  col:9 
implicit e 'int'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} '' 'struct C::(anonymous at 
{{.*}}:[[@LINE-8]]:3)'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} 'e' 'int'
+  // CHECK-NEXT: IndirectFieldDecl 0x{{[^ ]*}}  col:12 implicit f 'int'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} '' 'struct C::(anonymous at 
{{.*}}:[[@LINE-11]]:3)'
+  // CHECK-NEXT: Field 0x{{[^ ]*}} 'f' 'int'
+};
+
+struct D {
+  // CHECK-NEXT: RecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:8 struct D definition
+  int a;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 a 
'int'
+  int b[10];
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 b 
'int [10]'
+  int c[];
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 c 
'int []'
+};
+
+union E;
+// CHECK: RecordDecl 0x{{[^ ]*}}  col:7 union E
+
+union F;
+// CHECK: RecordDecl 0x{{[^ ]*}}  col:7 union F
+
+union E {
+  // CHECK: RecordDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}}  line:[[@LINE-1]]:7 union E definition
+  int a;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 a 
'int'
+  int b, c;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 b 
'int'
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:10 c 'int'
+  int d : 12;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:7 d 
'int'
+  // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 12
+  int : 0;
+  // CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:3 'int'
+  // CHECK-NEXT: Consta

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:238
+  // Apply -fdebug-prefix-map.
+  PP.RemapFilePaths = true;
+  PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };

Unconditionally?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

@ilya-biryukov 
What do you think about the global settable bool in MemoryBuffer in place of 
the ifdef from https://reviews.llvm.org/D35200 ? In this case the client on 
Windows can set it and you're safe that any MemoryBuffer call never mmaps.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54995/new/

https://reviews.llvm.org/D54995



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Can't say I know much abouth the path remapping functionality - what it's used 
for, where it's implemented in general, etc - so figure someone with more of 
that knowledge might be best off reviewing this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[George Karpenkov via cfe-commits]

Pushed the fix. Again, sorry for the trouble.

> On Nov 30, 2018, at 11:34 AM, George Karpenkov via cfe-commits 
>  wrote:
> 
> Good idea about using the mailing list, thanks, I’ll do that!
> 
>> On Nov 30, 2018, at 11:30 AM, Aaron Ballman  wrote:
>> 
>> On Fri, Nov 30, 2018 at 2:26 PM George Karpenkov  
>> wrote:
>>> 
>>> Thanks I’ll take a look.
>>> 
>>> BTW when reverting could you use “git revert” or mention manually the 
>>> phabricator revision being reverted,
>>> and apply reverts atomically?
>>> I (and many others) work exclusively using a git monorepo, so I don’t even 
>>> have a straightforward way to lookup what "r347951” is.
>> 
>> Given that I work exclusively in svn, I won't be using git revert. :-)
>> I can add the phab revision to the commit log when possible, but I
>> expect to continue to use svn revisions until the git transition takes
>> place. FWIW, you can use the mailing lists to look up what r347951
>> (such as 
>> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20181126/252804.html).
>> Sorry for the troubles, though!
>> 
>> ~Aaron
>> 
>>> 
>>> Thanks!
>>> 
 On Nov 30, 2018, at 11:20 AM, Aaron Ballman  wrote:
 
 On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov  
 wrote:
> 
> Thanks and sorry about the trouble. I’ll recommit with size_t.
 
 No worries, it happens! FYI, I also had to commit r348023 as part of
 the reverts.
 
 ~Aaron
> 
> On Nov 30, 2018, at 10:56 AM, Aaron Ballman  
> wrote:
> 
> On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
> cfe-commits  wrote:
> 
> 
> NoQ added inline comments.
> 
> 
> 
> Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
> +
> +  static void * operator new(unsigned long size);
> +
> 
> NoQ wrote:
> 
> I think we should use `size_t` as much as possible, because this may 
> otherwise have weird consequences on platforms on which `size_t` is not 
> defined as `unsigned long`. Not sure if this checker is ran on such 
> platforms. But the test doesn't have the triple specified, so it runs 
> under the host triple, which may be arbitrary and cause problems on 
> buildbots.
> 
> I.e.,
> 
> ```
> typedef __typeof(sizeof(int)) size_t;
> // use size_t
> ```
> 
> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
> 
> 
> I reverted r347949 through r347951 in r348020 to get the bots back to 
> green.
> 
> ~Aaron
> 
> 
> 
> Repository:
> rC Clang
> 
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D55076/new/
> 
> https://reviews.llvm.org/D55076
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 
> 
>>> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348031: [analyzer] Evaluate all non-checker config options 
before analysis (authored by Szelethus, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53692?vs=172856&id=176183#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53692/new/

https://reviews.llvm.org/D53692

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp

Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -204,7 +204,7 @@
 PP(CI.getPreprocessor()), OutDir(outdir), Opts(std::move(opts)),
 Plugins(plugins), Injector(injector), CTU(CI) {
 DigestAnalyzerOptions();
-if (Opts->PrintStats || Opts->shouldSerializeStats()) {
+if (Opts->PrintStats || Opts->ShouldSerializeStats) {
   AnalyzerTimers = llvm::make_unique(
   "analyzer", "Analyzer timers");
   TUTotalTimer = llvm::make_unique(
@@ -739,7 +739,7 @@
 
   // Execute the worklist algorithm.
   Eng.ExecuteWorkList(Mgr->getAnalysisDeclContextManager().getStackFrame(D),
-  Mgr->options.getMaxNodesPerTopLevelFunction());
+  Mgr->options.MaxNodesPerTopLevelFunction);
 
   if (!Mgr->options.DumpExplodedGraphTo.empty())
 Eng.DumpGraph(Mgr->options.TrimGraph, Mgr->options.DumpExplodedGraphTo);
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ lib/StaticAnalyzer/Frontend/ModelInjector.cpp
@@ -48,7 +48,7 @@
   FileID mainFileID = SM.getMainFileID();
 
   AnalyzerOptionsRef analyzerOpts = CI.getAnalyzerOpts();
-  llvm::StringRef modelPath = analyzerOpts->getModelPath();
+  llvm::StringRef modelPath = analyzerOpts->ModelPath;
 
   llvm::SmallString<128> fileName;
 
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -385,7 +385,7 @@
   // instead of generating an Unknown value and propagate the taint info to it.
   const unsigned MaxComp = StateMgr.getOwningEngine()
->getAnalysisManager()
-   .options.getMaxSymbolComplexity();
+   .options.MaxSymbolComplexity;
 
   if (symLHS && symRHS &&
   (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -676,8 +676,8 @@
 bool EnableNullFPSuppression, BugReport &BR,
 const SVal V) {
 AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
-if (EnableNullFPSuppression && Options.shouldSuppressNullReturnPaths()
-  && V.getAs())
+if (EnableNullFPSuppression &&
+Options.ShouldSuppressNullReturnPaths && V.getAs())
   BR.addVisitor(llvm::make_unique(
   R->getAs(), V));
   }
@@ -808,7 +808,8 @@
 AnalyzerOptions &Options = State->getAnalysisManager().options;
 
 bool EnableNullFPSuppression = false;
-if (InEnableNullFPSuppression && Options.shouldSuppressNullReturnPaths())
+if (InEnableNullFPSuppression &&
+Options.ShouldSuppressNullReturnPaths)
   if (Optional RetLoc = RetVal.getAs())
 EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
@@ -877,7 +878,7 @@
 // future nodes. We want to emit a path note as well, in case
 // the report is resurrected as valid later on.
 if (EnableNullFPSuppression &&
-Options.shouldAvoidSuppressingNullArgumentPaths())
+   

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[Artem Dergachev via cfe-commits]

Hmm, they don't have git-svn-id lines in the monorepo, which are handy 
for exactly that.


In the multirepos i usually just /search the git log for the svn commit 
(without "r") and it quickly matches the git-svn-id line, eg.:


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@347951 // <== 
this last part


On 11/30/18 11:34 AM, George Karpenkov wrote:

Good idea about using the mailing list, thanks, I’ll do that!


On Nov 30, 2018, at 11:30 AM, Aaron Ballman  wrote:

On Fri, Nov 30, 2018 at 2:26 PM George Karpenkov  wrote:

Thanks I’ll take a look.

BTW when reverting could you use “git revert” or mention manually the 
phabricator revision being reverted,
and apply reverts atomically?
I (and many others) work exclusively using a git monorepo, so I don’t even have a 
straightforward way to lookup what "r347951” is.

Given that I work exclusively in svn, I won't be using git revert. :-)
I can add the phab revision to the commit log when possible, but I
expect to continue to use svn revisions until the git transition takes
place. FWIW, you can use the mailing lists to look up what r347951
(such as 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20181126/252804.html).
Sorry for the troubles, though!

~Aaron


Thanks!


On Nov 30, 2018, at 11:20 AM, Aaron Ballman  wrote:

On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov  wrote:

Thanks and sorry about the trouble. I’ll recommit with size_t.

No worries, it happens! FYI, I also had to commit r348023 as part of
the reverts.

~Aaron

On Nov 30, 2018, at 10:56 AM, Aaron Ballman  wrote:

On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
cfe-commits  wrote:


NoQ added inline comments.



Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
+
+  static void * operator new(unsigned long size);
+

NoQ wrote:

I think we should use `size_t` as much as possible, because this may otherwise 
have weird consequences on platforms on which `size_t` is not defined as 
`unsigned long`. Not sure if this checker is ran on such platforms. But the 
test doesn't have the triple specified, so it runs under the host triple, which 
may be arbitrary and cause problems on buildbots.

I.e.,

```
typedef __typeof(sizeof(int)) size_t;
// use size_t
```

http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp


I reverted r347949 through r347951 in r348020 to get the bots back to green.

~Aaron



Repository:
rC Clang

CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55076/new/

https://reviews.llvm.org/D55076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits




___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348032 - Revert r348029. I was git-ing and jumped the gun.

[Bill Wendling via cfe-commits]

Author: void
Date: Fri Nov 30 12:44:11 2018
New Revision: 348032

URL: http://llvm.org/viewvc/llvm-project?rev=348032&view=rev
Log:
Revert r348029. I was git-ing and jumped the gun.

Modified:
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/test/CodeGen/builtin-constant-p.c

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=348032&r1=348031&r2=348032&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Fri Nov 30 12:44:11 2018
@@ -1552,8 +1552,7 @@ llvm::Constant *ConstantEmitter::tryEmit
   if (destType->isReferenceType())
 Success = E->EvaluateAsLValue(Result, CGM.getContext());
   else
-Success = E->EvaluateAsRValue(Result, CGM.getContext(),
-  /* InConstantContext */ true);
+Success = E->EvaluateAsRValue(Result, CGM.getContext());
 
   llvm::Constant *C;
   if (Success && !Result.HasSideEffects)

Modified: cfe/trunk/test/CodeGen/builtin-constant-p.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-constant-p.c?rev=348032&r1=348031&r2=348032&view=diff
==
--- cfe/trunk/test/CodeGen/builtin-constant-p.c (original)
+++ cfe/trunk/test/CodeGen/builtin-constant-p.c Fri Nov 30 12:44:11 2018
@@ -157,14 +157,3 @@ static void src_fn(void) {
 void test14() {
   assign(dest_p, src_fn);
 }
-
-struct test15_s {
-  const char *name;
-  int num_args;
-};
-
-extern int test15_v;
-
-struct test15_s tcg_op_defs_org_x86_64[] = {
-{"tag", __builtin_constant_p(test15_v) && !test15_v ? 0x10 : 0 },
-};


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348031 - [analyzer] Evaluate all non-checker config options before analysis

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Fri Nov 30 12:44:00 2018
New Revision: 348031

URL: http://llvm.org/viewvc/llvm-project?rev=348031&view=rev
Log:
[analyzer] Evaluate all non-checker config options before analysis

In earlier patches regarding AnalyzerOptions, a lot of effort went into
gathering all config options, and changing the interface so that potential
misuse can be eliminited.

Up until this point, AnalyzerOptions only evaluated an option when it was
querried. For example, if we had a "-no-false-positives" flag, AnalyzerOptions
would store an Optional field for it that would be None up until somewhere in
the code until the flag's getter function is called.

However, now that we're confident that we've gathered all configs, we can
evaluate off of them before analysis, so we can emit a error on invalid input
even if that prticular flag will not matter in that particular run of the
analyzer. Another very big benefit of this is that debug.ConfigDumper will now
show the value of all configs every single time.

Also, almost all options related class have a similar interface, so uniformity
is also a benefit.

The implementation for errors on invalid input will be commited shorty.

Differential Revision: https://reviews.llvm.org/D53692

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/analyzer-config.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def?rev=348031&r1=348030&r2=348031&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def Fri Nov 30 
12:44:00 2018
@@ -9,29 +9,6 @@
 //
 //  This file defines the analyzer options avaible with -analyzer-config.
 //
-//  This file is in part intended for method generation. If it's not included
-//  for that purpose, the following function-like macros should be predefined,
-//  through which all registered options are accessible:
-//
-//* ANALYZER_OPTION: Register a new option.
-//* ANALYZER_OPTION_DEPENDS_ON_USER_MODE: Register a new option, default
-//  value depends on the "user-mode" option.
-//
-//  Options where a simple getter method is sufficient are registered with the
-//  following macros:
-//
-//* ANALYZER_OPTION_GEN_FN: Register a new option, and generate a getter
-//  method for it in AnalyzerOptions.
-//
-//* ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE: Same as above, but
-//  generates a getter function that depends on the "user-mode" option.
-//
-//  You can only include this file when both or none of the above two macros
-//  are defined!
-//  When they are defined, entries that do not generate functions  won't 
appear,
-//  and when they aren't, all entries are converted to ANALYZER_OPTION or to
-//  ANALYZER_OPTION_DEPENDS_ON_USER_MODE.
-//
 
//===--===//
 
 #ifndef LLVM_ADT_STRINGREF_H
@@ -53,22 +30,6 @@ define both 'ANALYZER_OPTION' and 'ANALY
 #endif
 #endif
 
-#ifdef ANALYZER_OPTION_GEN_FN
-#ifndef ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE
-#error If you include this file with the intent of generating functions, \
-define both 'ANALYZER_OPTION_GEN_FN' and \
-'ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE' macros!
-#endif
-#endif
-
-#ifdef ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE
-#ifndef ANALYZER_OPTION_GEN_FN
-#error If you include this file with the intent of generating functions, \
-define both 'ANALYZER_OPTION_GEN_FN' and \
-'ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE' macros!
-#endif
-#endif
-
 #ifndef ANALYZER_OPTION
 /// Create a new analyzer option, but 

r348030 - Revert "Reverting r347949-r347951 because they broke the test bots."

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Nov 30 12:43:42 2018
New Revision: 348030

URL: http://llvm.org/viewvc/llvm-project?rev=348030&view=rev
Log:
Revert "Reverting r347949-r347951 because they broke the test bots."

This reverts commit 5bad6129c012fbf186eb055be49344e790448ecc.

Hopefully fixing the issue which was breaking the bots.

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=348030&r1=348029&r2=348030&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Fri Nov 
30 12:43:42 2018
@@ -530,6 +530,8 @@ class RetainSummaryManager {
   /// Decrement the reference count on OS object.
   const RetainSummary *getOSSummaryReleaseRule(const FunctionDecl *FD);
 
+  /// Free the OS object.
+  const RetainSummary *getOSSummaryFreeRule(const FunctionDecl *FD);
 
   enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable };
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=348030&r1=348029&r2=348030&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Fri Nov 30 
12:43:42 2018
@@ -124,10 +124,8 @@ RetainSummaryManager::generateSummary(co
   }
 
   const IdentifierInfo *II = FD->getIdentifier();
-  if (!II)
-return getDefaultSummary();
 
-  StringRef FName = II->getName();
+  StringRef FName = II ? II->getName() : "";
 
   // Strip away preceding '_'.  Doing this here will effect all the checks
   // down below.
@@ -304,6 +302,12 @@ RetainSummaryManager::generateSummary(co
 
   if (FName == "retain")
 return getOSSummaryRetainRule(FD);
+
+  if (FName == "free")
+return getOSSummaryFreeRule(FD);
+
+  if (MD->getOverloadedOperator() == OO_New)
+return getOSSummaryCreateRule(MD);
 }
   }
 
@@ -480,20 +484,14 @@ RetainSummaryManager::getSummary(const C
   const RetainSummary *Summ;
   switch (Call.getKind()) {
   case CE_Function:
-Summ = getFunctionSummary(cast(Call).getDecl());
-break;
   case CE_CXXMember:
-Summ = getFunctionSummary(cast(Call).getDecl());
-break;
   case CE_CXXMemberOperator:
-Summ = getFunctionSummary(cast(Call).getDecl());
-break;
   case CE_CXXConstructor:
-Summ = getFunctionSummary(cast(Call).getDecl());
+  case CE_CXXAllocator:
+Summ = getFunctionSummary(cast_or_null(Call.getDecl()));
 break;
   case CE_Block:
   case CE_CXXDestructor:
-  case CE_CXXAllocator:
 // FIXME: These calls are currently unsupported.
 return getPersistentStopSummary();
   case CE_ObjCMessage: {
@@ -619,6 +617,14 @@ RetainSummaryManager::getOSSummaryReleas
 }
 
 const RetainSummary *
+RetainSummaryManager::getOSSummaryFreeRule(const FunctionDecl *FD) {
+  return getPersistentSummary(RetEffect::MakeNoRet(),
+  /*ReceiverEff=*/DoNothing,
+  /*DefaultEff=*/DoNothing,
+  /*ThisEff=*/Dealloc);
+}
+
+const RetainSummary *
 RetainSummaryManager::getOSSummaryCreateRule(const FunctionDecl *FD) {
   return getPersistentSummary(RetEffect::MakeOwned(RetEffect::OS));
 }

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=348030&r1=348029&r2=348030&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Fri Nov 30 12:43:42 2018
@@ -11,9 +11,12 @@ struct OSMetaClass;
 #define OSDynamicCast(type, inst)   \
 ((type *) OSMetaClassBase::safeMetaCast((inst), OSTypeID(type)))
 
+using size_t = decltype(sizeof(int));
+
 struct OSObject {
   virtual void retain();
   virtual void release() {};
+  virtual void free();
   virtual ~OSObject(){}
 
   unsigned int foo() { return 42; }
@@ -23,6 +26,9 @@ struct OSObject {
   static OSObject *getObject();
   static OSObject *GetObject();
 
+
+  static void * operator new(size_t size);
+
   static const OSMetaClass * const metaClass;
 };
 
@@ -62,6 +68,34 @@ struct OSMetaClassBase {
   static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta);
 };
 
+void check_free_no_error() {
+  OSArray *arr = OSArray::withCapacity(

r348029 - We're in a constant context in the ConstantEmitter.

[Bill Wendling via cfe-commits]

Author: void
Date: Fri Nov 30 12:40:06 2018
New Revision: 348029

URL: http://llvm.org/viewvc/llvm-project?rev=348029&view=rev
Log:
We're in a constant context in the ConstantEmitter.

Modified:
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/test/CodeGen/builtin-constant-p.c

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=348029&r1=348028&r2=348029&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Fri Nov 30 12:40:06 2018
@@ -1552,7 +1552,8 @@ llvm::Constant *ConstantEmitter::tryEmit
   if (destType->isReferenceType())
 Success = E->EvaluateAsLValue(Result, CGM.getContext());
   else
-Success = E->EvaluateAsRValue(Result, CGM.getContext());
+Success = E->EvaluateAsRValue(Result, CGM.getContext(),
+  /* InConstantContext */ true);
 
   llvm::Constant *C;
   if (Success && !Result.HasSideEffects)

Modified: cfe/trunk/test/CodeGen/builtin-constant-p.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-constant-p.c?rev=348029&r1=348028&r2=348029&view=diff
==
--- cfe/trunk/test/CodeGen/builtin-constant-p.c (original)
+++ cfe/trunk/test/CodeGen/builtin-constant-p.c Fri Nov 30 12:40:06 2018
@@ -157,3 +157,14 @@ static void src_fn(void) {
 void test14() {
   assign(dest_p, src_fn);
 }
+
+struct test15_s {
+  const char *name;
+  int num_args;
+};
+
+extern int test15_v;
+
+struct test15_s tcg_op_defs_org_x86_64[] = {
+{"tag", __builtin_constant_p(test15_v) && !test15_v ? 0x10 : 0 },
+};


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov marked an inline comment as done.
george.karpenkov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp:483-497
   case CE_Function:
 Summ = getFunctionSummary(cast(Call).getDecl());
 break;
   case CE_CXXMember:
 Summ = getFunctionSummary(cast(Call).getDecl());
 break;
   case CE_CXXMemberOperator:

NoQ wrote:
> It's actually just `Call.getDecl()` and you can turn this into a fall-through.
Call.getDecl() returns a Decl (gotta love Obj-C methods!).

I guess we can group all those cases, and cast the returned decl to 
FunctionDecl instead of casting the call.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55076/new/

https://reviews.llvm.org/D55076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53153: [OpenCL] Mark kernel functions with default visibility

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D53153#1315107 , @yaxunl wrote:

> In D53153#1315039 , @scott.linder 
> wrote:
>
> > In D53153#1314798 , @rjmccall 
> > wrote:
> >
> > > You still have the same linkage model for those other languages, right?  
> > > Ultimately there's something like a kernel function that has to be 
> > > declared specially and which becomes the only public interface of the 
> > > code?
> >
> >
> > I don't think this is true for all languages targeting AMDGPU. For example, 
> > HIP has APIs like `hipMemcpyToSymbol` which accept a string identifying a 
> > symbol for a variable in global/constant address space. @yaxunl is my 
> > understanding about the HIP runtime requiring dynamic symbols for global 
> > variables correct here?
>
>
> Right. There is also hipGetSymbolAddress which returns address of a global 
> variable in device memory given the name of the variable. Therefore we cannot 
> hide the global variables.


Okay.  So it's still the case that all symbols will be defined within the 
linkage unit; it's just that some things might need to get exposed outside of 
it.

LLVM does provide a `dso_local` attribute which you could use unconditionally 
on every symbol without actually changing visibility.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53153/new/

https://reviews.llvm.org/D53153



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53153: [OpenCL] Mark kernel functions with default visibility

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D53153#1315039 , @scott.linder 
wrote:

> In D53153#1314798 , @rjmccall wrote:
>
> > You still have the same linkage model for those other languages, right?  
> > Ultimately there's something like a kernel function that has to be declared 
> > specially and which becomes the only public interface of the code?
>
>
> I don't think this is true for all languages targeting AMDGPU. For example, 
> HIP has APIs like `hipMemcpyToSymbol` which accept a string identifying a 
> symbol for a variable in global/constant address space. @yaxunl is my 
> understanding about the HIP runtime requiring dynamic symbols for global 
> variables correct here?


Right. There is also hipGetSymbolAddress which returns address of a global 
variable in device memory given the name of the variable. Therefore we cannot 
hide the global variables.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53153/new/

https://reviews.llvm.org/D53153



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Add myself as code owner for OpenBSD driver

[Richard Smith via cfe-commits]

LGTM

Thanks, welcome aboard :)

On Thu, 29 Nov 2018, 06:01 Brad Smith via cfe-commits <
cfe-commits@lists.llvm.org wrote:

> Add myself as code owner for OpenBSD driver.
>
>
> Index: CODE_OWNERS.TXT
> ===
> --- CODE_OWNERS.TXT (revision 347715)
> +++ CODE_OWNERS.TXT (working copy)
> @@ -49,6 +49,10 @@
>  E: rjmcc...@apple.com
>  D: Clang LLVM IR generation
>
> +N: Brad Smith
> +E: b...@comstyle.com
> +D: OpenBSD driver
> +
>  N: Richard Smith
>  E: rich...@metafoo.co.uk
>  D: All parts of Clang not covered by someone else
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D54995#1315024 , @zturner wrote:

> We can work around it by reading the whole file, but it looks like a bug in 
> QtCreator to me.  We have the file mapped, but maybe when they open the file 
> to save it, *they* are opening the file without `FILE_SHARE_READ`.


I haven't tried QtCreator, but this also happens in VSCode and Vim. I suspect 
other editors are also affected, but I haven't checked.
I looked at the logs in Process Monitor and I think they do set the share flags 
properly (~95% certain, will still have to double-check). Even if they do it 
wrong, it will take time to fix this and I don't think we can afford breaking 
users with existing editors in our tooling.

That being said, I don't think the "isVolatile" parameter should be present in 
the filesystem APIs. I propose to handle that at the `vfs::FileSystem` 
implementation itself that is used underneath.
D55139  is an initial fix for clangd that 
wraps existing `RealFileSystem` to force all files being open as volatile. It's 
much more reliable than passing the flags around, but it has a downside of also 
not memory-mapping **all** files, not just user files. However, I would not 
bother to try saving those bits unless we know that can actually critical for 
performance or memory usage (which I doubt would matter in the editors 
use-case, given that we still need to run the compiler on the files that we've 
read).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54995/new/

https://reviews.llvm.org/D54995



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348028 - Expect mixed path separators in FileManagerTest when resolving paths on Win32

[Matthew Voss via cfe-commits]

Author: ormris
Date: Fri Nov 30 11:52:50 2018
New Revision: 348028

URL: http://llvm.org/viewvc/llvm-project?rev=348028&view=rev
Log:
Expect mixed path separators in FileManagerTest when resolving paths on Win32

Modified:
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=348028&r1=348027&r2=348028&view=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Fri Nov 30 11:52:50 2018
@@ -363,7 +363,7 @@ TEST_F(FileManagerTest, getVirtualFileFi
   ASSERT_TRUE(file->isValid());
   SmallString<64> ExpectedResult;
 #ifdef _WIN32
-  ExpectedResult = "C:";
+  ExpectedResult = "C:/";
 #else
   ExpectedResult = "/";
 #endif


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, mgorny.

Memory-mapping files on Windows leads to them being locked and prevents
editors from saving changes to those files on disk. This is fine for the
compiler, but not acceptable for an interactive tool like clangd.
Therefore, we choose to avoid using memory-mapped files on Windows.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55139

Files:
  clangd/CMakeLists.txt
  clangd/FSProvider.cpp
  clangd/FSProvider.h

Index: clangd/FSProvider.h
===
--- clangd/FSProvider.h
+++ clangd/FSProvider.h
@@ -33,9 +33,7 @@
 public:
   // FIXME: returns the single real FS instance, which is not threadsafe.
   llvm::IntrusiveRefCntPtr
-  getFileSystem() const override {
-return llvm::vfs::getRealFileSystem();
-  }
+  getFileSystem() const override;
 };
 
 } // namespace clangd
Index: clangd/FSProvider.cpp
===
--- /dev/null
+++ clangd/FSProvider.cpp
@@ -0,0 +1,74 @@
+//===--- FSProvider.cpp - VFS provider for ClangdServer ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FSProvider.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// Always opens files in the underlying filesystem as "volatile", meaning they
+/// won't be memory-mapped. This avoid locking the files on Windows.
+class VolatileFSProvider : public llvm::vfs::ProxyFileSystem {
+public:
+  explicit VolatileFSProvider(llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr>
+  openFileForRead(const Twine &Path) override {
+auto File = getUnderlyingFS().openFileForRead(Path);
+if (!File)
+  return File;
+return std::unique_ptr(
+new VolatileFile(std::move(std::move(*File;
+  }
+
+private:
+  class VolatileFile : public vfs::File {
+  public:
+VolatileFile(std::unique_ptr Wrapped)
+: Wrapped(std::move(Wrapped)) {
+  assert(this->Wrapped);
+}
+
+virtual llvm::ErrorOr>
+getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+  bool /*IsVolatile*/) override {
+  return Wrapped->getBuffer(Name, FileSize, RequiresNullTerminator,
+/*IsVolatile=*/true);
+}
+
+llvm::ErrorOr status() override { return Wrapped->status(); }
+llvm::ErrorOr getName() override { return Wrapped->getName(); }
+std::error_code close() override { return Wrapped->close(); }
+
+  private:
+std::unique_ptr Wrapped;
+  };
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr
+clang::clangd::RealFileSystemProvider::getFileSystem() const {
+// Avoid using memory-mapped files on Windows, they cause file locking issues.
+// FIXME: Try to use a similar approach in Sema instead of relying on
+//error-prone propagation of the 'isVolatile' flag through all layers.
+#ifdef _WIN32
+  return new VolatileFSProvider(vfs::getRealFileSystem());
+#else
+  return vfs::getRealFileSystem();
+#endif
+}
+} // namespace clangd
+} // namespace clang
Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -23,6 +23,7 @@
   FindSymbols.cpp
   FileDistance.cpp
   FS.cpp
+  FSProvider.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be 
an issue there?




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59
+}
+}
+

`// end of namespace llvm`



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:210
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO pass the SourceLocation of the CallExpression for more precise
+// diagnostics

`TODO: `



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:214
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();
+// TODO Add statistics here
+return llvm::make_error(index_error_code::triple_mismatch);

Use punctuation.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55134/new/

https://reviews.llvm.org/D55134



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r348020 - Reverting r347949-r347951 because they broke the test bots.

[David Green via cfe-commits]

Thanks for the fix, And thanks for the work keeping the bots green!

Dave





From: Aaron Ballman 
Sent: 30 November 2018 19:35
To: David Green
Cc: cfe-commits; nd
Subject: Re: r348020 - Reverting r347949-r347951 because they broke the test 
bots.
  

On Fri, Nov 30, 2018 at 2:31 PM David Green  wrote:
>
> Hello!
>
>
> Did you intend to change the .gitignore here too?

No! That's a great catch, sorry about that -- VS integration with
CMake is rather broken without that manual change to .gitignore. :-(
I've restored it in r348026 -- sorry for the churn there!

~Aaron

>
>
> Cheers
>
> Dave
>
> 
> From: cfe-commits  on behalf of Aaron 
> Ballman via cfe-commits 
> Sent: 30 November 2018 18:52:51
> To: cfe-commits@lists.llvm.org
> Subject: r348020 - Reverting r347949-r347951 because they broke the test bots.
>
> Author: aaronballman
> Date: Fri Nov 30 10:52:51 2018
> New Revision: 348020
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348020&view=rev
> Log:
> Reverting r347949-r347951 because they broke the test bots.
>
>  
> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
>
> Modified:
> cfe/trunk/.gitignore
> cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
> cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
> cfe/trunk/test/Analysis/osobject-retain-release.cpp
> cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
>
> Modified: cfe/trunk/.gitignore
> URL:  
> http://llvm.org/viewvc/llvm-project/cfe/trunk/.gitignore?rev=348020&r1=348019&r2=348020&view=diff
> ==
> --- cfe/trunk/.gitignore (original)
> +++ cfe/trunk/.gitignore Fri Nov 30 10:52:51 2018
> @@ -29,8 +29,6 @@ cscope.out
>  
>#==#
>  # Directories to ignore (do not add trailing '/'s, they skip symlinks).
>  
>#==#
> -# Clang extra user tools, which is tracked independently (clang-tools-extra).
> -tools/extra
>  # Sphinx build products
>  docs/_build
>  docs/analyzer/_build
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
> URL:  
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=348020&r1=348019&r2=348020&view=diff
> ==
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
> (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Fri 
> Nov 30 10:52:51 2018
> @@ -530,8 +530,6 @@ class RetainSummaryManager {
>    /// Decrement the reference count on OS object.
>    const RetainSummary *getOSSummaryReleaseRule(const FunctionDecl *FD);
>
> -  /// Free the OS object.
> -  const RetainSummary *getOSSummaryFreeRule(const FunctionDecl *FD);
>
>    enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable 
>};
>
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
> URL:  
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=348020&r1=348019&r2=348020&view=diff
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Fri Nov 30 
> 10:52:51 2018
> @@ -124,8 +124,10 @@ RetainSummaryManager::generateSummary(co
>    }
>
>    const IdentifierInfo *II = FD->getIdentifier();
> +  if (!II)
> +    return getDefaultSummary();
>
> -  StringRef FName = II ? II->getName() : "";
> +  StringRef FName = II->getName();
>
>    // Strip away preceding '_'.  Doing this here will effect all the checks
>    // down below.
> @@ -302,12 +304,6 @@ RetainSummaryManager::generateSummary(co
>
>    if (FName == "retain")
>  return getOSSummaryRetainRule(FD);
> -
> -  if (FName == "free")
> -    return getOSSummaryFreeRule(FD);
> -
> -  if (MD->getOverloadedOperator() == OO_New)
> -    return getOSSummaryCreateRule(MD);
>  }
>    }
>
> @@ -495,11 +491,9 @@ RetainSummaryManager::getSummary(const C
>    case CE_CXXConstructor:
>  Summ = getFunctionSummary(cast(Call).getDecl());
>  break;
> -  case CE_CXXAllocator:
> -    Summ = getFunctionSummary(cast(Call).getDecl());
> -    break;
>    case CE_Block:
>    case CE_CXXDestructor:
> +  case CE_CXXAllocator:
>  // FIXME: These calls are currently unsupported.
>  return getPersistentStopSummary();
>    case CE_ObjCMessage: {
> @@ -625,14 +619,6 @@ RetainSummaryManager::getOSSummaryReleas
>  }
>
>  const RetainSummary *
> -RetainSummaryManager::getOSSummaryFreeRule(const FunctionDecl *FD) {
> -  return getPersistentSummary(Re

[PATCH] D53153: [OpenCL] Mark kernel functions with default visibility

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

In D53153#1314798 , @rjmccall wrote:

> You still have the same linkage model for those other languages, right?  
> Ultimately there's something like a kernel function that has to be declared 
> specially and which becomes the only public interface of the code?


I don't think this is true for all languages targeting AMDGPU. For example, HIP 
has APIs like `hipMemcpyToSymbol` which accept a string identifying a symbol 
for a variable in global/constant address space. @yaxunl is my understanding 
about the HIP runtime requiring dynamic symbols for global variables correct 
here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53153/new/

https://reviews.llvm.org/D53153



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[George Karpenkov via cfe-commits]

Good idea about using the mailing list, thanks, I’ll do that!

> On Nov 30, 2018, at 11:30 AM, Aaron Ballman  wrote:
> 
> On Fri, Nov 30, 2018 at 2:26 PM George Karpenkov  wrote:
>> 
>> Thanks I’ll take a look.
>> 
>> BTW when reverting could you use “git revert” or mention manually the 
>> phabricator revision being reverted,
>> and apply reverts atomically?
>> I (and many others) work exclusively using a git monorepo, so I don’t even 
>> have a straightforward way to lookup what "r347951” is.
> 
> Given that I work exclusively in svn, I won't be using git revert. :-)
> I can add the phab revision to the commit log when possible, but I
> expect to continue to use svn revisions until the git transition takes
> place. FWIW, you can use the mailing lists to look up what r347951
> (such as 
> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20181126/252804.html).
> Sorry for the troubles, though!
> 
> ~Aaron
> 
>> 
>> Thanks!
>> 
>>> On Nov 30, 2018, at 11:20 AM, Aaron Ballman  wrote:
>>> 
>>> On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov  
>>> wrote:
 
 Thanks and sorry about the trouble. I’ll recommit with size_t.
>>> 
>>> No worries, it happens! FYI, I also had to commit r348023 as part of
>>> the reverts.
>>> 
>>> ~Aaron
 
 On Nov 30, 2018, at 10:56 AM, Aaron Ballman  wrote:
 
 On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
 cfe-commits  wrote:
 
 
 NoQ added inline comments.
 
 
 
 Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
 +
 +  static void * operator new(unsigned long size);
 +
 
 NoQ wrote:
 
 I think we should use `size_t` as much as possible, because this may 
 otherwise have weird consequences on platforms on which `size_t` is not 
 defined as `unsigned long`. Not sure if this checker is ran on such 
 platforms. But the test doesn't have the triple specified, so it runs 
 under the host triple, which may be arbitrary and cause problems on 
 buildbots.
 
 I.e.,
 
 ```
 typedef __typeof(sizeof(int)) size_t;
 // use size_t
 ```
 
 http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
 
 
 I reverted r347949 through r347951 in r348020 to get the bots back to 
 green.
 
 ~Aaron
 
 
 
 Repository:
 rC Clang
 
 CHANGES SINCE LAST ACTION
 https://reviews.llvm.org/D55076/new/
 
 https://reviews.llvm.org/D55076
 
 
 
 ___
 cfe-commits mailing list
 cfe-commits@lists.llvm.org
 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
 
 
>> 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348026 - Revert an inadvertent change from r348020.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Nov 30 11:32:35 2018
New Revision: 348026

URL: http://llvm.org/viewvc/llvm-project?rev=348026&view=rev
Log:
Revert an inadvertent change from r348020.

Modified:
cfe/trunk/.gitignore

Modified: cfe/trunk/.gitignore
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/.gitignore?rev=348026&r1=348025&r2=348026&view=diff
==
--- cfe/trunk/.gitignore (original)
+++ cfe/trunk/.gitignore Fri Nov 30 11:32:35 2018
@@ -29,6 +29,8 @@ cscope.out
 
#==#
 # Directories to ignore (do not add trailing '/'s, they skip symlinks).
 
#==#
+# Clang extra user tools, which is tracked independently (clang-tools-extra).
+tools/extra
 # Sphinx build products
 docs/_build
 docs/analyzer/_build


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r348020 - Reverting r347949-r347951 because they broke the test bots.

[Aaron Ballman via cfe-commits]

On Fri, Nov 30, 2018 at 2:31 PM David Green  wrote:
>
> Hello!
>
>
> Did you intend to change the .gitignore here too?

No! That's a great catch, sorry about that -- VS integration with
CMake is rather broken without that manual change to .gitignore. :-(
I've restored it in r348026 -- sorry for the churn there!

~Aaron

>
>
> Cheers
>
> Dave
>
> 
> From: cfe-commits  on behalf of Aaron 
> Ballman via cfe-commits 
> Sent: 30 November 2018 18:52:51
> To: cfe-commits@lists.llvm.org
> Subject: r348020 - Reverting r347949-r347951 because they broke the test bots.
>
> Author: aaronballman
> Date: Fri Nov 30 10:52:51 2018
> New Revision: 348020
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348020&view=rev
> Log:
> Reverting r347949-r347951 because they broke the test bots.
>
> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
>
> Modified:
> cfe/trunk/.gitignore
> cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
> cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
> cfe/trunk/test/Analysis/osobject-retain-release.cpp
> cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
>
> Modified: cfe/trunk/.gitignore
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/.gitignore?rev=348020&r1=348019&r2=348020&view=diff
> ==
> --- cfe/trunk/.gitignore (original)
> +++ cfe/trunk/.gitignore Fri Nov 30 10:52:51 2018
> @@ -29,8 +29,6 @@ cscope.out
>  
> #==#
>  # Directories to ignore (do not add trailing '/'s, they skip symlinks).
>  
> #==#
> -# Clang extra user tools, which is tracked independently (clang-tools-extra).
> -tools/extra
>  # Sphinx build products
>  docs/_build
>  docs/analyzer/_build
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=348020&r1=348019&r2=348020&view=diff
> ==
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
> (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Fri 
> Nov 30 10:52:51 2018
> @@ -530,8 +530,6 @@ class RetainSummaryManager {
>/// Decrement the reference count on OS object.
>const RetainSummary *getOSSummaryReleaseRule(const FunctionDecl *FD);
>
> -  /// Free the OS object.
> -  const RetainSummary *getOSSummaryFreeRule(const FunctionDecl *FD);
>
>enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable 
> };
>
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=348020&r1=348019&r2=348020&view=diff
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Fri Nov 30 
> 10:52:51 2018
> @@ -124,8 +124,10 @@ RetainSummaryManager::generateSummary(co
>}
>
>const IdentifierInfo *II = FD->getIdentifier();
> +  if (!II)
> +return getDefaultSummary();
>
> -  StringRef FName = II ? II->getName() : "";
> +  StringRef FName = II->getName();
>
>// Strip away preceding '_'.  Doing this here will effect all the checks
>// down below.
> @@ -302,12 +304,6 @@ RetainSummaryManager::generateSummary(co
>
>if (FName == "retain")
>  return getOSSummaryRetainRule(FD);
> -
> -  if (FName == "free")
> -return getOSSummaryFreeRule(FD);
> -
> -  if (MD->getOverloadedOperator() == OO_New)
> -return getOSSummaryCreateRule(MD);
>  }
>}
>
> @@ -495,11 +491,9 @@ RetainSummaryManager::getSummary(const C
>case CE_CXXConstructor:
>  Summ = getFunctionSummary(cast(Call).getDecl());
>  break;
> -  case CE_CXXAllocator:
> -Summ = getFunctionSummary(cast(Call).getDecl());
> -break;
>case CE_Block:
>case CE_CXXDestructor:
> +  case CE_CXXAllocator:
>  // FIXME: These calls are currently unsupported.
>  return getPersistentStopSummary();
>case CE_ObjCMessage: {
> @@ -625,14 +619,6 @@ RetainSummaryManager::getOSSummaryReleas
>  }
>
>  const RetainSummary *
> -RetainSummaryManager::getOSSummaryFreeRule(const FunctionDecl *FD) {
> -  return getPersistentSummary(RetEffect::MakeNoRet(),
> -  /*ReceiverEff=*/DoNothing,
> -  /*DefaultEff=*/DoNothing,
> -  /*ThisEff=*/Dealloc);
> -}
> -
> -const RetainSummary *
>  RetainSummaryManager::ge

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176174.
aprantl added a comment.

fix a comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55137/new/

https://reviews.llvm.org/D55137

Files:
  include/clang/AST/PrettyPrinter.h
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-prefix-map-lambda.cpp

Index: test/CodeGenCXX/debug-prefix-map-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-prefix-map-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   -fdebug-prefix-map=%S=/SOURCE_ROOT %s -emit-llvm -o - | FileCheck %s
+
+template  void b(T) {}
+void c() {
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
+  b([]{});
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -341,6 +341,9 @@
 
   void finalize();
 
+  /// Remap a path using the -fdebug-prefix-map.
+  std::string remapDIPath(StringRef Path) const;
+
   /// Register VLA size expression debug node with the qualified type.
   void registerVLASizeExpression(QualType Ty, llvm::Metadata *SizeExpr) {
 SizeExprCache[Ty] = SizeExpr;
@@ -528,9 +531,6 @@
   /// Create new compile unit.
   void CreateCompileUnit();
 
-  /// Remap a given path with the current debug prefix map
-  std::string remapDIPath(StringRef) const;
-
   /// Compute the file checksum debug info for input file ID.
   Optional
   computeChecksum(FileID FID, SmallString<32> &Checksum) const;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -234,6 +234,9 @@
   if (CGM.getCodeGenOpts().EmitCodeView)
 PP.MSVCFormatting = true;
 
+  // Apply -fdebug-prefix-map.
+  PP.RemapFilePaths = true;
+  PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };
   return PP;
 }
 
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1157,9 +1157,13 @@
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -38,21 +38,20 @@
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(false), ConstantsAsWritten(false),
+SuppressImplicitBase(false), FullyQualifiedName(false),
+RemapFilePaths(false) {}
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: dblaikie, davide.
aprantl added a project: debug-info.

This adds a callback to `PrintingPolicy` to allow `CGDebugInfo` to remap file 
paths according to `-fdebug-prefix-map`. Otherwise the debug info (particularly 
function names for C++ lambdas) may contain paths that should have been 
remapped in the debug info.


https://reviews.llvm.org/D55137

Files:
  include/clang/AST/PrettyPrinter.h
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-prefix-map-lambda.cpp

Index: test/CodeGenCXX/debug-prefix-map-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-prefix-map-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   -fdebug-prefix-map=%S=/SOURCE_ROOT %s -emit-llvm -o - | FileCheck %s
+
+template  void b(T) {}
+void c() {
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
+  b([]{});
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -341,6 +341,9 @@
 
   void finalize();
 
+  /// Remap a given path with the current debug prefix map
+  std::string remapDIPath(StringRef) const;
+
   /// Register VLA size expression debug node with the qualified type.
   void registerVLASizeExpression(QualType Ty, llvm::Metadata *SizeExpr) {
 SizeExprCache[Ty] = SizeExpr;
@@ -528,9 +531,6 @@
   /// Create new compile unit.
   void CreateCompileUnit();
 
-  /// Remap a given path with the current debug prefix map
-  std::string remapDIPath(StringRef) const;
-
   /// Compute the file checksum debug info for input file ID.
   Optional
   computeChecksum(FileID FID, SmallString<32> &Checksum) const;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -234,6 +234,9 @@
   if (CGM.getCodeGenOpts().EmitCodeView)
 PP.MSVCFormatting = true;
 
+  // Apply -fdebug-prefix-map.
+  PP.RemapFilePaths = true;
+  PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };
   return PP;
 }
 
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1157,9 +1157,13 @@
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -38,21 +38,20 @@
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(f

Re: r348020 - Reverting r347949-r347951 because they broke the test bots.

[David Green via cfe-commits]

Hello!


Did you intend to change the .gitignore here too?


Cheers

Dave


From: cfe-commits  on behalf of Aaron 
Ballman via cfe-commits 
Sent: 30 November 2018 18:52:51
To: cfe-commits@lists.llvm.org
Subject: r348020 - Reverting r347949-r347951 because they broke the test bots.

Author: aaronballman
Date: Fri Nov 30 10:52:51 2018
New Revision: 348020

URL: http://llvm.org/viewvc/llvm-project?rev=348020&view=rev
Log:
Reverting r347949-r347951 because they broke the test bots.

http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp

Modified:
cfe/trunk/.gitignore
cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test

Modified: cfe/trunk/.gitignore
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/.gitignore?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/.gitignore (original)
+++ cfe/trunk/.gitignore Fri Nov 30 10:52:51 2018
@@ -29,8 +29,6 @@ cscope.out
 
#==#
 # Directories to ignore (do not add trailing '/'s, they skip symlinks).
 
#==#
-# Clang extra user tools, which is tracked independently (clang-tools-extra).
-tools/extra
 # Sphinx build products
 docs/_build
 docs/analyzer/_build

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Fri Nov 
30 10:52:51 2018
@@ -530,8 +530,6 @@ class RetainSummaryManager {
   /// Decrement the reference count on OS object.
   const RetainSummary *getOSSummaryReleaseRule(const FunctionDecl *FD);

-  /// Free the OS object.
-  const RetainSummary *getOSSummaryFreeRule(const FunctionDecl *FD);

   enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable };


Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Fri Nov 30 
10:52:51 2018
@@ -124,8 +124,10 @@ RetainSummaryManager::generateSummary(co
   }

   const IdentifierInfo *II = FD->getIdentifier();
+  if (!II)
+return getDefaultSummary();

-  StringRef FName = II ? II->getName() : "";
+  StringRef FName = II->getName();

   // Strip away preceding '_'.  Doing this here will effect all the checks
   // down below.
@@ -302,12 +304,6 @@ RetainSummaryManager::generateSummary(co

   if (FName == "retain")
 return getOSSummaryRetainRule(FD);
-
-  if (FName == "free")
-return getOSSummaryFreeRule(FD);
-
-  if (MD->getOverloadedOperator() == OO_New)
-return getOSSummaryCreateRule(MD);
 }
   }

@@ -495,11 +491,9 @@ RetainSummaryManager::getSummary(const C
   case CE_CXXConstructor:
 Summ = getFunctionSummary(cast(Call).getDecl());
 break;
-  case CE_CXXAllocator:
-Summ = getFunctionSummary(cast(Call).getDecl());
-break;
   case CE_Block:
   case CE_CXXDestructor:
+  case CE_CXXAllocator:
 // FIXME: These calls are currently unsupported.
 return getPersistentStopSummary();
   case CE_ObjCMessage: {
@@ -625,14 +619,6 @@ RetainSummaryManager::getOSSummaryReleas
 }

 const RetainSummary *
-RetainSummaryManager::getOSSummaryFreeRule(const FunctionDecl *FD) {
-  return getPersistentSummary(RetEffect::MakeNoRet(),
-  /*ReceiverEff=*/DoNothing,
-  /*DefaultEff=*/DoNothing,
-  /*ThisEff=*/Dealloc);
-}
-
-const RetainSummary *
 RetainSummaryManager::getOSSummaryCreateRule(const FunctionDecl *FD) {
   return getPersistentSummary(RetEffect::MakeOwned(RetEffect::OS));
 }

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/oso

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176172.
martong added a comment.

- Add lit tests


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55134/new/

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,16 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection  -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -28,6 +28,36 @@
 #include 
 #include 
 
+namespace llvm {
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const Triple &Lhs, const Triple &Rhs) {
+  return ((Lhs.getArch() != Triple::UnknownArch &&
+   Rhs.getArch() != Triple::UnknownArch)
+  ? Lhs.getArch() == Rhs.getArch()
+  : true) &&
+ ((Lhs.getSubArch() != Triple::NoSubArch &&
+   Rhs.getSubArch() != Triple::NoSubArch)
+  ? Lhs.getSubArch() == Rhs.getSubArch()
+  : true) &&
+ ((Lhs.getVendor() != Triple::UnknownVendor &&
+   Rhs.getVendor() != Triple::UnknownVendor)
+  ? Lhs.getVendor() == Rhs.getVendor()
+  : true) &&
+ ((Lhs.getOS() != Triple::UnknownOS && Rhs.getOS() != Triple::UnknownOS)
+  ? Lhs.getOS() == Rhs.getOS()
+  : true) &&
+ ((Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+   Rhs.getEnvironment() != Triple::UnknownEnvironment)
+  ? Lhs.getEnvironment() == Rhs.getEnvironment()
+  : true) &&
+ ((Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+   Rhs.getObjectFormat() != Triple::UnknownObjectFormat)
+  ? Lhs.getObjectFormat() == Rhs.getObjectFormat()
+  : true);
+}
+}
+
 namespace clang {
 namespace cross_tu {
 
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(&Unit->getFileManager() ==
  &Unit->getASTContext().getSourceManager().getFileManager());
 
+  const auto& TripleTo = Context.getTargetInfo().getTriple();
+  const auto& TripleFrom = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO pass the SourceLocation of the CallExpression for more precise
+// diagnostics
+Context.getDiagnostics().Report(dia

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[Aaron Ballman via cfe-commits]

On Fri, Nov 30, 2018 at 2:26 PM George Karpenkov  wrote:
>
> Thanks I’ll take a look.
>
> BTW when reverting could you use “git revert” or mention manually the 
> phabricator revision being reverted,
> and apply reverts atomically?
> I (and many others) work exclusively using a git monorepo, so I don’t even 
> have a straightforward way to lookup what "r347951” is.

Given that I work exclusively in svn, I won't be using git revert. :-)
I can add the phab revision to the commit log when possible, but I
expect to continue to use svn revisions until the git transition takes
place. FWIW, you can use the mailing lists to look up what r347951
(such as 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20181126/252804.html).
Sorry for the troubles, though!

~Aaron

>
> Thanks!
>
> > On Nov 30, 2018, at 11:20 AM, Aaron Ballman  wrote:
> >
> > On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov  
> > wrote:
> >>
> >> Thanks and sorry about the trouble. I’ll recommit with size_t.
> >
> > No worries, it happens! FYI, I also had to commit r348023 as part of
> > the reverts.
> >
> > ~Aaron
> >>
> >> On Nov 30, 2018, at 10:56 AM, Aaron Ballman  wrote:
> >>
> >> On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
> >> cfe-commits  wrote:
> >>
> >>
> >> NoQ added inline comments.
> >>
> >>
> >> 
> >> Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
> >> +
> >> +  static void * operator new(unsigned long size);
> >> +
> >> 
> >> NoQ wrote:
> >>
> >> I think we should use `size_t` as much as possible, because this may 
> >> otherwise have weird consequences on platforms on which `size_t` is not 
> >> defined as `unsigned long`. Not sure if this checker is ran on such 
> >> platforms. But the test doesn't have the triple specified, so it runs 
> >> under the host triple, which may be arbitrary and cause problems on 
> >> buildbots.
> >>
> >> I.e.,
> >>
> >> ```
> >> typedef __typeof(sizeof(int)) size_t;
> >> // use size_t
> >> ```
> >>
> >> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
> >>
> >>
> >> I reverted r347949 through r347951 in r348020 to get the bots back to 
> >> green.
> >>
> >> ~Aaron
> >>
> >>
> >>
> >> Repository:
> >> rC Clang
> >>
> >> CHANGES SINCE LAST ACTION
> >> https://reviews.llvm.org/D55076/new/
> >>
> >> https://reviews.llvm.org/D55076
> >>
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>
> >>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

We can work around it by reading the whole file, but it looks like a bug in 
QtCreator to me.  We have the file mapped, but maybe when they open the file to 
save it, *they* are opening the file without `FILE_SHARE_READ`.  It's a logical 
thing to do on the surface, because they are probably thinking "we are about to 
change the contents of the file, so we don't want anyone to be reading it at 
the same time while we modify it".  But this means their call to `CreateFile()` 
will fail, because we have the file opened for read, and if they want to open 
it for exclusive access, it's not possible.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54995/new/

https://reviews.llvm.org/D54995



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[George Karpenkov via cfe-commits]

Thanks I’ll take a look.

BTW when reverting could you use “git revert” or mention manually the 
phabricator revision being reverted,
and apply reverts atomically?
I (and many others) work exclusively using a git monorepo, so I don’t even have 
a straightforward way to lookup what "r347951” is.

Thanks!

> On Nov 30, 2018, at 11:20 AM, Aaron Ballman  wrote:
> 
> On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov  wrote:
>> 
>> Thanks and sorry about the trouble. I’ll recommit with size_t.
> 
> No worries, it happens! FYI, I also had to commit r348023 as part of
> the reverts.
> 
> ~Aaron
>> 
>> On Nov 30, 2018, at 10:56 AM, Aaron Ballman  wrote:
>> 
>> On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
>> cfe-commits  wrote:
>> 
>> 
>> NoQ added inline comments.
>> 
>> 
>> 
>> Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
>> +
>> +  static void * operator new(unsigned long size);
>> +
>> 
>> NoQ wrote:
>> 
>> I think we should use `size_t` as much as possible, because this may 
>> otherwise have weird consequences on platforms on which `size_t` is not 
>> defined as `unsigned long`. Not sure if this checker is ran on such 
>> platforms. But the test doesn't have the triple specified, so it runs under 
>> the host triple, which may be arbitrary and cause problems on buildbots.
>> 
>> I.e.,
>> 
>> ```
>> typedef __typeof(sizeof(int)) size_t;
>> // use size_t
>> ```
>> 
>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
>> 
>> 
>> I reverted r347949 through r347951 in r348020 to get the bots back to green.
>> 
>> ~Aaron
>> 
>> 
>> 
>> Repository:
>> rC Clang
>> 
>> CHANGES SINCE LAST ACTION
>> https://reviews.llvm.org/D55076/new/
>> 
>> https://reviews.llvm.org/D55076
>> 
>> 
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> 
>> 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52988: [analyzer][PlistMacroExpansion] Part 5.: Support for # and ##

[Umann Kristóf via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348025: [analyzer][PlistMacroExpansion] Part 5.: Support for 
# and ## (authored by Szelethus, committed by ).
Herald added subscribers: llvm-commits, gamesh411.

Changed prior to commit:
  https://reviews.llvm.org/D52988?vs=170073&id=176170#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52988/new/

https://reviews.llvm.org/D52988

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp

Index: cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
===
--- cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
+++ cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
@@ -345,9 +345,17 @@
   *ptr = 5; // expected-warning{{Dereference of null pointer}}
 }
 
-// TODO: Should expand correctly.
 // CHECK: nameDECLARE_FUNC_AND_SET_TO_NULL
-// CHECK-NEXT: expansionvoid generated_##whatever(); ptr = nullptr;
+// CHECK-NEXT: expansionvoid generated_whatever(); ptr = nullptr;
+
+void macroArgContainsHashHashInStringTest() {
+  int *a;
+  TO_NULL_AND_PRINT(a, "Will this ## cause a crash?");
+  *a = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameTO_NULL_AND_PRINT
+// CHECK-NEXT: expansiona = 0; print( "Will this ## cause a crash?")
 
 #define PRINT_STR(str, ptr) \
   print(#str);  \
@@ -359,9 +367,17 @@
   *ptr = 5; // expected-warning{{Dereference of null pointer}}
 }
 
-// TODO: Should expand correctly.
 // CHECK: namePRINT_STR
-// CHECK-NEXT: expansionprint(#Hello); ptr = nullptr
+// CHECK-NEXT: expansionprint("Hello"); ptr = nullptr
+
+void macroArgContainsHashInStringTest() {
+  int *a;
+  TO_NULL_AND_PRINT(a, "Will this # cause a crash?");
+  *a = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameTO_NULL_AND_PRINT
+// CHECK-NEXT: expansiona = 0; print( "Will this # cause a crash?")
 
 //===--===//
 // Tests for more complex macro expansions.
Index: cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -4555,7 +4555,7 @@
   file0
  
  nameDECLARE_FUNC_AND_SET_TO_NULL
- expansionvoid generated_##whatever(); ptr = nullptr;
+ expansionvoid generated_whatever(); ptr = nullptr;
 

descriptionDereference of null pointer (loaded from variable 'ptr')
@@ -4595,12 +4595,12 @@
 start
  
   
-   line357
+   line352
col3
file0
   
   
-   line357
+   line352
col5
file0
   
@@ -4608,12 +4608,181 @@
 end
  
   
-   line358
+   line353
col3
file0
   
   
-   line358
+   line353
+   col19
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line353
+  col3
+  file0
+ 
+ ranges
+ 
+   
+
+ line353
+ col3
+ file0
+
+
+ line353
+ col53
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Null pointer value stored to 'a'
+ message
+ Null pointer value stored to 'a'
+
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line354
+   col3
+   file0
+  
+  
+   line354
+   col3
+   file0
+  
+ 
+end
+ 
+  
+   line354
+   col6
+   file0
+  
+  
+   line354
+   col6
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line354
+  col6
+  file0
+ 
+ ranges
+ 
+   
+
+ line354
+ col4
+ file0
+
+
+ line354
+ col4
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Dereference of null pointer (loaded from variable 'a')
+ message
+ Dereference of null pointer (loaded from variable 'a')
+
+   
+   macro_expansions
+   
+
+ location
+ 
+  line353
+  col3
+  file0
+ 
+ nameTO_NULL_AND_PRINT
+ expansiona = 0; print( "Will this ## cause a crash?")
+
+   
+   descriptionDereference of null pointer (loaded from variable 'a')
+   

[PATCH] D53329: Generate DIFile with main program if source is not available

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

The Verifier patch has landed as https://reviews.llvm.org/rL348022

I do not have any more knowledge than the reviewers of the right way to handle 
providing source for remapped files, but at least compilation will warn and 
strip debug-info rather than segfault when source is provided inconsistently.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53329/new/

https://reviews.llvm.org/D53329



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348025 - [analyzer][PlistMacroExpansion] Part 5.: Support for # and ##

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Fri Nov 30 11:21:35 2018
New Revision: 348025

URL: http://llvm.org/viewvc/llvm-project?rev=348025&view=rev
Log:
[analyzer][PlistMacroExpansion] Part 5.: Support for # and ##

From what I can see, this should be the last patch needed to replicate macro
argument expansions.

Differential Revision: https://reviews.llvm.org/D52988

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=348025&r1=348024&r2=348025&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Fri Nov 30 11:21:35 
2018
@@ -904,8 +904,6 @@ static std::string getMacroNameAndPrintE
   continue;
 }
 
-// TODO: Handle tok::hash and tok::hashhash.
-
 // If control reached here, then this token isn't a macro identifier, nor 
an
 // unexpanded macro argument that we need to handle, print it.
 Printer.printToken(T);
@@ -1094,14 +1092,25 @@ void MacroArgMap::expandFromPrevMacro(co
 }
 
 void TokenPrinter::printToken(const Token &Tok) {
-  // If the tokens were already space separated, or if they must be to avoid
-  // them being implicitly pasted, add a space between them.
   // If this is the first token to be printed, don't print space.
-  if (PrevTok.isNot(tok::unknown) && (Tok.hasLeadingSpace() ||
-  ConcatInfo.AvoidConcat(PrevPrevTok, PrevTok, Tok)))
-OS << ' ';
+  if (PrevTok.isNot(tok::unknown)) {
+// If the tokens were already space separated, or if they must be to avoid
+// them being implicitly pasted, add a space between them.
+if(Tok.hasLeadingSpace() || ConcatInfo.AvoidConcat(PrevPrevTok, PrevTok,
+   Tok)) {
+  // AvoidConcat doesn't check for ##, don't print a space around it.
+  if (PrevTok.isNot(tok::hashhash) && Tok.isNot(tok::hashhash)) {
+OS << ' ';
+  }
+}
+  }
 
-  OS << PP.getSpelling(Tok);
+  if (!Tok.isOneOf(tok::hash, tok::hashhash)) {
+if (PrevTok.is(tok::hash))
+  OS << '\"' << PP.getSpelling(Tok) << '\"';
+else
+  OS << PP.getSpelling(Tok);
+  }
 
   PrevPrevTok = PrevTok;
   PrevTok = Tok;

Modified: 
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist?rev=348025&r1=348024&r2=348025&view=diff
==
--- 
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
 (original)
+++ 
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
 Fri Nov 30 11:21:35 2018
@@ -4555,7 +4555,7 @@
   file0
  
  nameDECLARE_FUNC_AND_SET_TO_NULL
- expansionvoid generated_##whatever(); ptr = 
nullptr;
+ expansionvoid generated_whatever(); ptr = 
nullptr;
 

descriptionDereference of null pointer (loaded from 
variable 'ptr')
@@ -4595,12 +4595,12 @@
 start
  
   
-   line357
+   line352
col3
file0
   
   
-   line357
+   line352
col5
file0
   
@@ -4608,12 +4608,181 @@
 end
  
   
-   line358
+   line353
col3
file0
   
   
-   line358
+   line353
+   col19
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line353
+  col3
+  file0
+ 
+ ranges
+ 
+   
+
+ line353
+ col3
+ file0
+
+
+ line353
+ col53
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Null pointer value stored to 'a'
+ message
+ Null pointer value stored to 'a'
+
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line354
+   col3
+   file0
+  
+  
+   line354
+   col3
+   file0
+  
+ 
+end
+ 
+  
+   line354
+   col6
+   file0
+  
+  
+   line354
+   col6
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line354
+  col6
+  file0
+ 
+ ranges
+ 
+   
+
+ line354
+ col4
+ file0
+ 

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D54995#1314912 , @zturner wrote:

> Original patch description says this:
>
> > There are reported cases of non-system files being locked by libclang on 
> > Windows (and likely by other clients as well)
>
> What is the nature of the reports?  What operation is attempted on the files 
> and fails due to locking?  And which application is it that's failing and in 
> what way?


https://bugreports.qt.io/browse/QTCREATORBUG-15449 was mentioned previously in 
the thread.

I've tried reproducing with the provided example and succeeded: if I map a view 
of the file, none of the editors would be able to write into that file.
I tried various combinations of flags, trying really hard to do readonly and 
copy-on-write mapping, but none would work, a file cannot be written into while 
there are active mapped views of the it.

E.g. for VSCode process monitor reports it eventually tries to call 
SetEndOfFile 

 and fails with `ERROR_USER_MAPPED_FILE` error (it can be found in the list of 
errors 

 from Microsoft).
There's this also Stackoverflow question 

 mentioning some cases where this occurs.

It looks like there's no workaround for this case in the Windows APIs, so I 
guess we'll have to trade-off memory in that case and read the whole file using 
the classical file APIs.

@yvvan, I was proven wrong. Thanks for taking your time to go through the 
trouble of providing this example. I'm not sure why my previous experiments 
came up with a different result, I've probably made a mistake somewhere along 
the way.
Nevertheless, could we avoid changing the defaults to `true`? We probably 
forgot to propagate the flag somewhere, those places should be easy to find by 
**temporarily** removing the default args, rerunning the build for libclang and 
then bringing the default args back.
It's fine to enable this behavior for editors, but various command-line tools, 
including the compiler, are definitely better off using the memory-mapped files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54995/new/

https://reviews.llvm.org/D54995



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[Aaron Ballman via cfe-commits]

On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov  wrote:
>
> Thanks and sorry about the trouble. I’ll recommit with size_t.

No worries, it happens! FYI, I also had to commit r348023 as part of
the reverts.

~Aaron
>
> On Nov 30, 2018, at 10:56 AM, Aaron Ballman  wrote:
>
> On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
> cfe-commits  wrote:
>
>
> NoQ added inline comments.
>
>
> 
> Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
> +
> +  static void * operator new(unsigned long size);
> +
> 
> NoQ wrote:
>
> I think we should use `size_t` as much as possible, because this may 
> otherwise have weird consequences on platforms on which `size_t` is not 
> defined as `unsigned long`. Not sure if this checker is ran on such 
> platforms. But the test doesn't have the triple specified, so it runs under 
> the host triple, which may be arbitrary and cause problems on buildbots.
>
> I.e.,
>
> ```
> typedef __typeof(sizeof(int)) size_t;
> // use size_t
> ```
>
> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
>
>
> I reverted r347949 through r347951 in r348020 to get the bots back to green.
>
> ~Aaron
>
>
>
> Repository:
>  rC Clang
>
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D55076/new/
>
> https://reviews.llvm.org/D55076
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[George Karpenkov via cfe-commits]

Thanks and sorry about the trouble. I’ll recommit with size_t.

> On Nov 30, 2018, at 10:56 AM, Aaron Ballman  wrote:
> 
> On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
> cfe-commits mailto:cfe-commits@lists.llvm.org>> 
> wrote:
>> 
>> NoQ added inline comments.
>> 
>> 
>> 
>> Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
>> +
>> +  static void * operator new(unsigned long size);
>> +
>> 
>> NoQ wrote:
>>> I think we should use `size_t` as much as possible, because this may 
>>> otherwise have weird consequences on platforms on which `size_t` is not 
>>> defined as `unsigned long`. Not sure if this checker is ran on such 
>>> platforms. But the test doesn't have the triple specified, so it runs under 
>>> the host triple, which may be arbitrary and cause problems on buildbots.
>>> 
>>> I.e.,
>>> 
>>> ```
>>> typedef __typeof(sizeof(int)) size_t;
>>> // use size_t
>>> ```
>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp
> 
> I reverted r347949 through r347951 in r348020 to get the bots back to green.
> 
> ~Aaron
> 
>> 
>> 
>> Repository:
>>  rC Clang
>> 
>> CHANGES SINCE LAST ACTION
>>  https://reviews.llvm.org/D55076/new/ 
>> 
>> https://reviews.llvm.org/D55076 
>> 
>> 
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org 
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
>> 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348023 - Updating this test, which changed after the reverts from r348020.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Nov 30 11:15:07 2018
New Revision: 348023

URL: http://llvm.org/viewvc/llvm-project?rev=348023&view=rev
Log:
Updating this test, which changed after the reverts from r348020.

Modified:
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test

Modified: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test?rev=348023&r1=348022&r2=348023&view=diff
==
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test 
(original)
+++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test Fri Nov 
30 11:15:07 2018
@@ -2,7 +2,7 @@
 
 // The number of supported attributes should never go down!
 
-// CHECK: #pragma clang attribute supports 132 attributes:
+// CHECK: #pragma clang attribute supports 133 attributes:
 // CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
@@ -86,8 +86,8 @@
 // CHECK-NEXT: NoThrow (SubjectMatchRule_function)
 // CHECK-NEXT: NotTailCalled (SubjectMatchRule_function)
 // CHECK-NEXT: OSConsumed (SubjectMatchRule_variable_is_parameter)
-// CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
-// CHECK-NEXT: OSReturnsRetained (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
+// CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, 
SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
+// CHECK-NEXT: OSReturnsRetained (SubjectMatchRule_function, 
SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: ObjCBoxable (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCBridge (SubjectMatchRule_record, 
SubjectMatchRule_type_alias)
 // CHECK-NEXT: ObjCBridgeMutable (SubjectMatchRule_record)


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: lib/IR/DiagnosticInfo.cpp:39
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 

aprantl wrote:
> probinson wrote:
> > Do we use a case-sensitive sort of include files?  I thought it was 
> > insensitive.  Of course the coding standard doesn't say. :-P
> I just ran clang-format without thinking about this...
"Don't argue with clang-format."  Okay.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55085/new/

https://reviews.llvm.org/D55085



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[Aaron Ballman via cfe-commits]

On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
cfe-commits  wrote:
>
> NoQ added inline comments.
>
>
> 
> Comment at: clang/test/Analysis/osobject-retain-release.cpp:27
> +
> +  static void * operator new(unsigned long size);
> +
> 
> NoQ wrote:
> > I think we should use `size_t` as much as possible, because this may 
> > otherwise have weird consequences on platforms on which `size_t` is not 
> > defined as `unsigned long`. Not sure if this checker is ran on such 
> > platforms. But the test doesn't have the triple specified, so it runs under 
> > the host triple, which may be arbitrary and cause problems on buildbots.
> >
> > I.e.,
> >
> > ```
> > typedef __typeof(sizeof(int)) size_t;
> > // use size_t
> > ```
> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp

I reverted r347949 through r347951 in r348020 to get the bots back to green.

~Aaron

>
>
> Repository:
>   rC Clang
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55076/new/
>
> https://reviews.llvm.org/D55076
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348020 - Reverting r347949-r347951 because they broke the test bots.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Nov 30 10:52:51 2018
New Revision: 348020

URL: http://llvm.org/viewvc/llvm-project?rev=348020&view=rev
Log:
Reverting r347949-r347951 because they broke the test bots.

http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp

Modified:
cfe/trunk/.gitignore
cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test

Modified: cfe/trunk/.gitignore
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/.gitignore?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/.gitignore (original)
+++ cfe/trunk/.gitignore Fri Nov 30 10:52:51 2018
@@ -29,8 +29,6 @@ cscope.out
 
#==#
 # Directories to ignore (do not add trailing '/'s, they skip symlinks).
 
#==#
-# Clang extra user tools, which is tracked independently (clang-tools-extra).
-tools/extra
 # Sphinx build products
 docs/_build
 docs/analyzer/_build

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Fri Nov 
30 10:52:51 2018
@@ -530,8 +530,6 @@ class RetainSummaryManager {
   /// Decrement the reference count on OS object.
   const RetainSummary *getOSSummaryReleaseRule(const FunctionDecl *FD);
 
-  /// Free the OS object.
-  const RetainSummary *getOSSummaryFreeRule(const FunctionDecl *FD);
 
   enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable };
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Fri Nov 30 
10:52:51 2018
@@ -124,8 +124,10 @@ RetainSummaryManager::generateSummary(co
   }
 
   const IdentifierInfo *II = FD->getIdentifier();
+  if (!II)
+return getDefaultSummary();
 
-  StringRef FName = II ? II->getName() : "";
+  StringRef FName = II->getName();
 
   // Strip away preceding '_'.  Doing this here will effect all the checks
   // down below.
@@ -302,12 +304,6 @@ RetainSummaryManager::generateSummary(co
 
   if (FName == "retain")
 return getOSSummaryRetainRule(FD);
-
-  if (FName == "free")
-return getOSSummaryFreeRule(FD);
-
-  if (MD->getOverloadedOperator() == OO_New)
-return getOSSummaryCreateRule(MD);
 }
   }
 
@@ -495,11 +491,9 @@ RetainSummaryManager::getSummary(const C
   case CE_CXXConstructor:
 Summ = getFunctionSummary(cast(Call).getDecl());
 break;
-  case CE_CXXAllocator:
-Summ = getFunctionSummary(cast(Call).getDecl());
-break;
   case CE_Block:
   case CE_CXXDestructor:
+  case CE_CXXAllocator:
 // FIXME: These calls are currently unsupported.
 return getPersistentStopSummary();
   case CE_ObjCMessage: {
@@ -625,14 +619,6 @@ RetainSummaryManager::getOSSummaryReleas
 }
 
 const RetainSummary *
-RetainSummaryManager::getOSSummaryFreeRule(const FunctionDecl *FD) {
-  return getPersistentSummary(RetEffect::MakeNoRet(),
-  /*ReceiverEff=*/DoNothing,
-  /*DefaultEff=*/DoNothing,
-  /*ThisEff=*/Dealloc);
-}
-
-const RetainSummary *
 RetainSummaryManager::getOSSummaryCreateRule(const FunctionDecl *FD) {
   return getPersistentSummary(RetEffect::MakeOwned(RetEffect::OS));
 }

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=348020&r1=348019&r2=348020&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Fri Nov 30 10:52:51 2018
@@ -14,7 +14,6 @@ struct OSMetaClass;
 struct OSObject {
   virtual void retain();
   virtual void release() {};
-  virtual void free();
   virtual ~OSObject(){}
 
   unsigned int foo() { return 42; }
@@ -24,9 +23,6 @@ struct OSObject {
   static OSObject 

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus requested changes to this revision.
Szelethus added a comment.
This revision now requires changes to proceed.

The idea is great!

I think this should rather be an -analyzer-config flag, since the actual 
analysis changes with a new output (refer to `AnalyzerOption`'s doxygen 
comments). Please note that I'm about to land some patches that modifies 
`AnalyzerOptions.def` quite a bit, but rebasing shouldn't be too bad anyways.

Also, almost everywhere CTU is capitalized, so I guess it should be in the 
field name too.




Comment at: test/Analysis/ctu-main.cpp:6
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-analyzer-display-ctu-progress 2>&1 %s | FileCheck %s
+

I think these RUN lines would really benefit from introducing line breaks.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55135/new/

https://reviews.llvm.org/D55135



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55023: OpenCL: Improve vector printf warnings

[Matt Arsenault via Phabricator via cfe-commits]

arsenm marked an inline comment as done.
arsenm added inline comments.



Comment at: test/SemaOpenCL/printf-format-strings.cl:65
+{
+printf("%v4f\n", arg); // expected-warning {{format specifies type 'double 
__attribute__((ext_vector_type(4)))' but the argument has type 'float2' (vector 
of 2 'float' values)}}
 }

Anastasia wrote:
> arsenm wrote:
> > Anastasia wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > arsenm wrote:
> > > > > > Anastasia wrote:
> > > > > > > So there is no way to print vector of float? What will happen on 
> > > > > > > architectures that don't support doubles?
> > > > > > > 
> > > > > > > I guess it's the same for printf in general with the float type?
> > > > > > There is, it's converted to a vector of doubles. This case warns 
> > > > > > because the element count mismatches
> > > > > The conversion is to float if doubles aren't supported
> > > > The warning message is wrong though without doubles, and still says 
> > > > double.
> > > Can we add this explicitly to the test just for a record? I guess the 
> > > warning message can't be changed to print the type correctly?
> > > 
> > > Also would it make sense to test other builtin types: double, char...?
> > This should be easy to fix, but the way to test for doubles is enabled is 
> > making this unnecessarily difficult. It requires threading OpenCLOptions 
> > through all of the relevant functions to check for cl_khr_fp64. Why is this 
> > separate from LangOptions?
> I see. I think the problem is that it is set conditionally based on the 
> targets in earlier CL versions. So it had to be a separated into target 
> specific option. :(
Would it be reasonable to add a new HasNoDouble field or something in 
LangOptions?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55023/new/

https://reviews.llvm.org/D55023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348015 - [clang] Fix rL348006 for windows

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Fri Nov 30 10:36:31 2018
New Revision: 348015

URL: http://llvm.org/viewvc/llvm-project?rev=348015&view=rev
Log:
[clang] Fix rL348006 for windows

Modified:
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=348015&r1=348014&r2=348015&view=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Fri Nov 30 10:36:31 2018
@@ -361,7 +361,14 @@ TEST_F(FileManagerTest, getVirtualFileFi
   const FileEntry *file = manager.getVirtualFile("/tmp/test", 123, 1);
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
-  EXPECT_EQ(file->tryGetRealPathName(), "/tmp/test");
+  SmallString<64> ExpectedResult;
+#ifdef _WIN32
+  ExpectedResult = "C:";
+#else
+  ExpectedResult = "/";
+#endif
+  llvm::sys::path::append(ExpectedResult, "tmp", "test");
+  EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
 
 } // anonymous namespace


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

>> Would turning asan operator new/delete into weak symbols help?
> 
> The new operator is already a weak symbol in libcxx, so it looks like an ODR 
> violation, doesn't it?

I don't think so. It's just symbol interposition. Every module in the process 
gets the same definition.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54905/new/

https://reviews.llvm.org/D54905



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52879: Derive builtin return type from its definition

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

Please see https://reviews.llvm.org/D55136 for the patch addressing these 
issues.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52879/new/

https://reviews.llvm.org/D52879



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55136: [OpenCL][Sema] Improve BuildResolvedCallExpr handling of builtins

[Marco Antognini via Phabricator via cfe-commits]

mantognini created this revision.
mantognini added a reviewer: riccibruno.
Herald added subscribers: cfe-commits, kristina, yaxunl.

This is a follow-up on https://reviews.llvm.org/D52879, addressing a few issues.

This:

- adds a FIXME for later improvement for specific builtins: I previously have 
only checked OpenCL ones and ensured tests cover those.
- fixed the CallExpr type.


Repository:
  rC Clang

https://reviews.llvm.org/D55136

Files:
  lib/Sema/SemaExpr.cpp


Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5549,17 +5549,19 @@
   // We special-case function promotion here because we only allow promoting
   // builtin functions to function pointers in the callee of a call.
   ExprResult Result;
-  QualType ReturnTy;
+  QualType ResultTy;
   if (BuiltinID &&
   Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
 // Extract the return type from the (builtin) function pointer type.
-auto FnPtrTy = Context.getPointerType(FDecl->getType());
+// FIXME Several builtins still have setType in 
Sema::CheckBuiltinFunctionCall.
+//   One should review their definitions in Builtins.def to ensure they
+//   are correct before removing setType calls.
+QualType FnPtrTy = Context.getPointerType(FDecl->getType());
 Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get();
-auto FnTy = FnPtrTy->getPointeeType()->castAs();
-ReturnTy = FnTy->getReturnType();
+ResultTy = FDecl->getCallResultType();
   } else {
 Result = CallExprUnaryConversions(Fn);
-ReturnTy = Context.BoolTy;
+ResultTy = Context.BoolTy;
   }
   if (Result.isInvalid())
 return ExprError();
@@ -5571,10 +5573,10 @@
   if (Config)
 TheCall =
 new (Context) CUDAKernelCallExpr(Context, Fn, cast(Config),
- Args, ReturnTy, VK_RValue, RParenLoc);
+ Args, ResultTy, VK_RValue, RParenLoc);
   else
 TheCall = new (Context)
-CallExpr(Context, Fn, Args, ReturnTy, VK_RValue, RParenLoc);
+CallExpr(Context, Fn, Args, ResultTy, VK_RValue, RParenLoc);
 
   if (!getLangOpts().CPlusPlus) {
 // C cannot always handle TypoExpr nodes in builtin calls and direct


Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5549,17 +5549,19 @@
   // We special-case function promotion here because we only allow promoting
   // builtin functions to function pointers in the callee of a call.
   ExprResult Result;
-  QualType ReturnTy;
+  QualType ResultTy;
   if (BuiltinID &&
   Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
 // Extract the return type from the (builtin) function pointer type.
-auto FnPtrTy = Context.getPointerType(FDecl->getType());
+// FIXME Several builtins still have setType in Sema::CheckBuiltinFunctionCall.
+//   One should review their definitions in Builtins.def to ensure they
+//   are correct before removing setType calls.
+QualType FnPtrTy = Context.getPointerType(FDecl->getType());
 Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get();
-auto FnTy = FnPtrTy->getPointeeType()->castAs();
-ReturnTy = FnTy->getReturnType();
+ResultTy = FDecl->getCallResultType();
   } else {
 Result = CallExprUnaryConversions(Fn);
-ReturnTy = Context.BoolTy;
+ResultTy = Context.BoolTy;
   }
   if (Result.isInvalid())
 return ExprError();
@@ -5571,10 +5573,10 @@
   if (Config)
 TheCall =
 new (Context) CUDAKernelCallExpr(Context, Fn, cast(Config),
- Args, ReturnTy, VK_RValue, RParenLoc);
+ Args, ResultTy, VK_RValue, RParenLoc);
   else
 TheCall = new (Context)
-CallExpr(Context, Fn, Args, ReturnTy, VK_RValue, RParenLoc);
+CallExpr(Context, Fn, Args, ResultTy, VK_RValue, RParenLoc);
 
   if (!getLangOpts().CPlusPlus) {
 // C cannot always handle TypoExpr nodes in builtin calls and direct
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

[Hyrum Wright via Phabricator via cfe-commits]

hwright added a comment.

Oh, and thanks for taking the time to review this. :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54737/new/

https://reviews.llvm.org/D54737



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl marked an inline comment as done.
aprantl added inline comments.



Comment at: lib/IR/DiagnosticInfo.cpp:39
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 

probinson wrote:
> Do we use a case-sensitive sort of include files?  I thought it was 
> insensitive.  Of course the coding standard doesn't say. :-P
I just ran clang-format without thinking about this...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55085/new/

https://reviews.llvm.org/D55085



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits