[PATCH] D136007: [clang][modules][deps] System module maps might not be affecting

2022-11-02 Thread Jan Svoboda via Phabricator via cfe-commits 
jansvoboda11 added a comment.

I'm not sure about that. The failing test isn't using modules and it has failed 
with the same error before: 
https://lab.llvm.org/buildbot/#/builders/237/builds/345


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136007

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

[PATCH] D136007: [clang][modules][deps] System module maps might not be affecting

2022-11-02 Thread Vitaly Buka via Phabricator via cfe-commits 
vitalybuka added subscribers: fmayer, vitalybuka.
vitalybuka added a comment.

FYI @fmayer

Probably this patch
https://lab.llvm.org/buildbot/#/builders/237/builds/350/steps/13/logs/stdio

  -- Testing: 66920 tests, 48 workers --
  Testing:  0.. 10.. 20
  FAIL: Clang :: SemaCXX/sugar-common-types.cpp (14838 of 66920)
   TEST 'Clang :: SemaCXX/sugar-common-types.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm_build_msan/bin/clang -cc1 
-internal-isystem 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm_build_msan/lib/clang/16.0.0/include
 -nostdsysteminc -fsyntax-only -verify 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/test/SemaCXX/sugar-common-types.cpp
 -std=c++20 -fenable-matrix -triple i686-pc-win32
  --
  Exit Code: 1
  Command Output (stderr):
  --
  ==3367002==WARNING: MemorySanitizer: use-of-uninitialized-value
  #0 0xd3ed5664 in 
clang::TemplateArgument::Profile(llvm::FoldingSetNodeID&, clang::ASTContext 
const&) const 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/TemplateBase.cpp:310:3
  #1 0xd3f77740 in clang::AutoType::Profile(llvm::FoldingSetNodeID&, 
clang::ASTContext const&, clang::QualType, clang::AutoTypeKeyword, bool, 
clang::ConceptDecl*, llvm::ArrayRef) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/Type.cpp:4532:9
  #2 0xd34ac920 in Profile 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/llvm/include/llvm/ADT/FoldingSet.h:268:7
  #3 0xd34ac920 in Equals 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/llvm/include/llvm/ADT/FoldingSet.h:420:3
  #4 0xd34ac920 in llvm::ContextualFoldingSet::NodeEquals(llvm::FoldingSetBase const*, 
llvm::FoldingSetBase::Node*, llvm::FoldingSetNodeID const&, unsigned int, 
llvm::FoldingSetNodeID&) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/llvm/include/llvm/ADT/FoldingSet.h:600:12
  #5 0xcd221610 in 
llvm::FoldingSetBase::FindNodeOrInsertPos(llvm::FoldingSetNodeID const&, 
void*&, llvm::FoldingSetBase::FoldingSetInfo const&) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/llvm/lib/Support/FoldingSet.cpp:288:9
  #6 0xd3450850 in FindNodeOrInsertPos 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/llvm/include/llvm/ADT/FoldingSet.h:490:45
  #7 0xd3450850 in 
clang::ASTContext::getAutoTypeInternal(clang::QualType, clang::AutoTypeKeyword, 
bool, bool, clang::ConceptDecl*, llvm::ArrayRef, bool) 
const 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ASTContext.cpp:5804:32
  #8 0xd3450e34 in clang::ASTContext::getAutoType(clang::QualType, 
clang::AutoTypeKeyword, bool, bool, clang::ConceptDecl*, 
llvm::ArrayRef) const 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ASTContext.cpp:5846:10
  #9 0xd2e44934 in (anonymous 
namespace)::SubstituteDeducedTypeTransform::TransformAutoType(clang::TypeLocBuilder&,
 clang::AutoTypeLoc) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaTemplateDeduction.cpp:4618:41
  #10 0xd2e407e4 in clang::TreeTransform<(anonymous 
namespace)::SubstituteDeducedTypeTransform>::TransformType(clang::TypeLocBuilder&,
 clang::TypeLoc) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm_build_msan/tools/clang/include/clang/AST/TypeNodes.inc:37:1
  #11 0xd2e22d94 in (anonymous 
namespace)::SubstituteDeducedTypeTransform::Apply(clang::TypeLoc) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaTemplateDeduction.cpp:4650:14
  #12 0xd2e225cc in clang::Sema::DeduceAutoType(clang::TypeLoc, 
clang::Expr*, clang::QualType&, clang::sema::TemplateDeductionInfo&, bool, 
bool) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaTemplateDeduction.cpp:4867:63
  #13 0xd1fd7c14 in 
clang::Sema::deduceVarTypeFromInitializer(clang::VarDecl*, 
clang::DeclarationName, clang::QualType, clang::TypeSourceInfo*, 
clang::SourceRange, bool, clang::Expr*) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaDecl.cpp:12481:7
  #14 0xd1fd85d8 in 
clang::Sema::DeduceVariableDeclarationType(clang::VarDecl*, bool, clang::Expr*) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaDecl.cpp:12517:26
  #15 0xd1fd99f8 in clang::Sema::AddInitializerToDecl(clang::Decl*, 
clang::Expr*, bool) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaDecl.cpp:12868:9
  #16 0xd1a605a8 in 
clang::Parser::ParseDeclarationAfterDeclaratorAndAttributes(clang::Declarator&, 
clang::Parser::ParsedTemplateInfo const&, clang::Parser::ForRangeInit*) 
/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/ParseDecl.cpp:2491:17
  #17 0xd1a5a844 in

[PATCH] D136007: [clang][modules][deps] System module maps might not be affecting

2022-11-01 Thread Jan Svoboda via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf33173acd687: [clang][modules][deps] System module maps 
might not be affecting (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D136007?vs=467979=472508#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136007

Files:
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/ClangScanDeps/modules-implicit-dot-private.m
  clang/test/ClangScanDeps/modules-incomplete-umbrella.c
  clang/test/ClangScanDeps/modules-redefinition.m

Index: clang/test/ClangScanDeps/modules-redefinition.m
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-redefinition.m
@@ -0,0 +1,60 @@
+// This test checks that we don't report non-affecting system module maps.
+// More specifically, we check that explicitly-specified module map file is not
+// being shadowed in explicit build by a module map file found during implicit
+// module map search.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- tu.m
+@import first;
+
+//--- zeroth/module.modulemap
+module X {}
+
+//--- first/module.modulemap
+module first { header "first.h" }
+//--- first/first.h
+@import third;
+//--- second/module.modulemap
+module X {}
+//--- third/module.modulemap
+module third {}
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.m",
+  "directory": "DIR",
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fmodule-name=X -fmodule-map-file=DIR/zeroth/module.modulemap -isystem DIR/first -isystem DIR/second -isystem DIR/third -c DIR/tu.m -o DIR/tu.o"
+}]
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+// RUN: cat %t/result.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "module-name": "third"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "clang-modulemap-file": "[[PREFIX]]/first/module.modulemap",
+// CHECK-NEXT:   "command-line": [
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK-NEXT: [[PREFIX]]/first/first.h",
+// CHECK-NEXT: [[PREFIX]]/first/module.modulemap",
+// CHECK-NEXT: [[PREFIX]]/third/module.modulemap"
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "name": "first"
+// CHECK-NEXT: }
+// CHECK:]
+// CHECK:  }
+
+// RUN: %deps-to-rsp %t/result.json --module-name=third > %t/third.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --module-name=first > %t/first.cc1.rsp
+// RUN: %clang @%t/third.cc1.rsp
+// RUN: %clang @%t/first.cc1.rsp
Index: clang/test/ClangScanDeps/modules-incomplete-umbrella.c
===
--- clang/test/ClangScanDeps/modules-incomplete-umbrella.c
+++ clang/test/ClangScanDeps/modules-incomplete-umbrella.c
@@ -26,7 +26,7 @@
 [{
   "file": "DIR/from_tu.m",
   "directory": "DIR",
-  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/from_tu.m -o DIR/from_tu.o"
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/from_tu.m -o DIR/from_tu.o"
 }]
 //--- from_tu.m
 #include "FW/FW.h"
@@ -45,8 +45,7 @@
 // CHECK_TU-NEXT:   "context-hash": "{{.*}}",
 // CHECK_TU-NEXT:   "file-deps": [
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
-// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
-// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
+// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
 // CHECK_TU-NEXT:   ],
 // CHECK_TU-NEXT:   "name": "FW"
 // CHECK_TU-NEXT: },
@@ -57,7 +56,6 @@
 // CHECK_TU:],
 // CHECK_TU-NEXT:   "context-hash": "{{.*}}",
 // CHECK_TU-NEXT:   "file-deps": [
-// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/One.h"
@@ -102,7 +100,7 @@
 [{
   "file": "DIR/from_module.m",
   "directory": "DIR",
-  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
 }]

[PATCH] D136007: [clang][modules][deps] System module maps might not be affecting

2022-10-26 Thread Michael Spencer via Phabricator via cfe-commits 
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

lgtm once D136624  lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136007

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

[PATCH] D136007: [clang][modules][deps] System module maps might not be affecting

2022-10-17 Thread Jan Svoboda via Phabricator via cfe-commits 
jansvoboda11 added inline comments.



Comment at: clang/test/ClangScanDeps/modules-redefinition.m:37
+// RUN: %clang @%t/third.cc1.rsp
+// RUN: %clang @%t/first.cc1.rsp

Before this patch, this would fail with duplicate definition of module 'X'. It 
would be reached from the explicitly-provided 
`-fmodule-map-file=%t/zeroth/module.modulemap` and the implicitly discovered 
`%t/second/module.modulemap`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136007

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

[PATCH] D136007: [clang][modules][deps] System module maps might not be affecting

2022-10-14 Thread Jan Svoboda via Phabricator via cfe-commits 
jansvoboda11 created this revision.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The dependency scanner relies on the module map filtering logic in `ASTWriter`. 
The algorithm currently considers all system module maps affecting, which is 
not only sub-optimal, but can also cause failures when building a module 
explicitly (see attached test case).

This patch applies the same filtering logic to system module maps.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136007

Files:
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/ClangScanDeps/modules-implicit-dot-private.m
  clang/test/ClangScanDeps/modules-incomplete-umbrella.c
  clang/test/ClangScanDeps/modules-redefinition.m

Index: clang/test/ClangScanDeps/modules-redefinition.m
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-redefinition.m
@@ -0,0 +1,37 @@
+// This test checks that we don't report (potentially) duplicate module maps
+// implicit build picked up from a system search directory.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- tu.m
+@import first; // this doesn't repro with "@import third;"
+
+//--- zeroth/module.modulemap
+module X {} // module name must match "-fmodule-name=" value
+
+//--- first/module.modulemap
+module first { header "first.h" }
+//--- first/first.h
+@import third;
+//--- second/module.modulemap
+module X {}
+//--- third/module.modulemap
+module third {}
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.m",
+  "directory": "DIR",
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fmodule-name=X -fmodule-map-file=DIR/zeroth/module.modulemap -isystem DIR/first -isystem DIR/second -isystem DIR/third -c DIR/tu.m -o DIR/tu.o"
+}]
+
+// RUN: %clang -fmodules -fmodules-cache-path=%t/cache -fmodule-name=X -fmodule-map-file=%t/zeroth/module.modulemap -isystem %t/first -isystem %t/second -isystem %t/third -c %t/tu.m -o %t/tu.o
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+// RUN: %deps-to-rsp %t/result.json --module-name=third > %t/third.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --module-name=first > %t/first.cc1.rsp
+// RUN: %clang @%t/third.cc1.rsp
+// RUN: %clang @%t/first.cc1.rsp
Index: clang/test/ClangScanDeps/modules-incomplete-umbrella.c
===
--- clang/test/ClangScanDeps/modules-incomplete-umbrella.c
+++ clang/test/ClangScanDeps/modules-incomplete-umbrella.c
@@ -45,8 +45,7 @@
 // CHECK_TU-NEXT:   "context-hash": "{{.*}}",
 // CHECK_TU-NEXT:   "file-deps": [
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
-// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
-// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
+// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
 // CHECK_TU-NEXT:   ],
 // CHECK_TU-NEXT:   "name": "FW"
 // CHECK_TU-NEXT: },
@@ -57,7 +56,6 @@
 // CHECK_TU:],
 // CHECK_TU-NEXT:   "context-hash": "{{.*}}",
 // CHECK_TU-NEXT:   "file-deps": [
-// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
 // CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/One.h"
@@ -102,7 +100,7 @@
 [{
   "file": "DIR/from_module.m",
   "directory": "DIR",
-  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
 }]
 //--- module.modulemap
 module Mod { header "Mod.h" }
@@ -125,8 +123,7 @@
 // CHECK_MODULE-NEXT:   "context-hash": "{{.*}}",
 // CHECK_MODULE-NEXT:   "file-deps": [
 // CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
-// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
-// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
+// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
 // CHECK_MODULE-NEXT:   ],
 // CHECK_MODULE-NEXT:   "name": "FW"
 // CHECK_MODULE-NEXT: },
@@ -137,7 +134,6 @@
 // CHECK_MODULE:],
 // CHECK_MODULE-NEXT:   "context-hash": "{{.*}}",
 // CHECK_MODULE-NEXT:   "file-deps": [
-// CHECK_MODULE-NEXT: